From b4ec6760148f5839e8c05247191ae055cac49252 Mon Sep 17 00:00:00 2001 From: ColinLee Date: Mon, 9 Nov 2026 09:57:17 +0800 Subject: [PATCH 1/4] [C++] Match Java bloom filter hash for non-ASCII paths Java sign-extends Murmur128Hash tail bytes while C++ zero-extended them, so the two disagree whenever a tail byte has its high bit set, i.e. for multi-byte UTF-8 device path segments. Bloom filter positions are persisted in the file, so the C++ reader computed positions that were never set, BloomFilter::contains() returned false and TsFileIOReader::alloc_ssi() silently dropped the series, leaving an all-NULL column. Hash with Java semantics when writing so C++-written files stay readable from Java, and accept either variant when reading so files written by older C++ releases stay readable. A bloom filter may return false positives, never false negatives, so checking both positions is safe. Refs #1077 --- cpp/src/common/container/murmur_hash3.cc | 46 +++++++++++------- cpp/src/common/container/murmur_hash3.h | 17 ++++++- cpp/src/reader/bloom_filter.cc | 22 +++++++-- cpp/src/reader/bloom_filter.h | 16 ++++++- cpp/test/reader/bloom_filter_test.cc | 60 ++++++++++++++++++++++++ 5 files changed, 136 insertions(+), 25 deletions(-) diff --git a/cpp/src/common/container/murmur_hash3.cc b/cpp/src/common/container/murmur_hash3.cc index 256542333f..d29cb4ba99 100644 --- a/cpp/src/common/container/murmur_hash3.cc +++ b/cpp/src/common/container/murmur_hash3.cc @@ -22,7 +22,21 @@ namespace common { /* ================ Murmur128Hash ================ */ // follow Java IoTDB exactly. -int64_t Murmur128Hash::inner_hash(const char* buf, int32_t len, int64_t seed) { +namespace { +// Java's byte is signed and its tail loop does `(long) key[i]`, so bytes with +// the high bit set contribute 0xFFFF...FF instead of 0x00...FF. +FORCE_INLINE uint64_t tail_byte(const char* buf, int32_t index, + bool signed_tail) { + if (signed_tail) { + return static_cast( + static_cast(static_cast(buf[index]))); + } + return static_cast(static_cast(buf[index])); +} +} // namespace + +int64_t Murmur128Hash::inner_hash(const char* buf, int32_t len, int64_t seed, + bool signed_tail) { const int32_t block_count = len >> 4; uint64_t h1 = static_cast(seed); uint64_t h2 = static_cast(seed); @@ -57,53 +71,53 @@ int64_t Murmur128Hash::inner_hash(const char* buf, int32_t len, int64_t seed) { uint64_t k2 = 0; switch (len & 15) { case 15: - k2 ^= (uint64_t)(uint8_t)buf[offset + 14] << 48; + k2 ^= tail_byte(buf, offset + 14, signed_tail) << 48; // fallthrough case 14: - k2 ^= (uint64_t)(uint8_t)buf[offset + 13] << 40; + k2 ^= tail_byte(buf, offset + 13, signed_tail) << 40; // fallthrough case 13: - k2 ^= (uint64_t)(uint8_t)buf[offset + 12] << 32; + k2 ^= tail_byte(buf, offset + 12, signed_tail) << 32; // fallthrough case 12: - k2 ^= (uint64_t)(uint8_t)buf[offset + 11] << 24; + k2 ^= tail_byte(buf, offset + 11, signed_tail) << 24; // fallthrough case 11: - k2 ^= (uint64_t)(uint8_t)buf[offset + 10] << 16; + k2 ^= tail_byte(buf, offset + 10, signed_tail) << 16; // fallthrough case 10: - k2 ^= (uint64_t)(uint8_t)buf[offset + 9] << 8; + k2 ^= tail_byte(buf, offset + 9, signed_tail) << 8; // fallthrough case 9: - k2 ^= (uint64_t)(uint8_t)buf[offset + 8]; + k2 ^= tail_byte(buf, offset + 8, signed_tail); k2 *= c2; k2 = rotl64(k2, 33); k2 *= c1; h2 ^= k2; // fallthrough case 8: - k1 ^= (uint64_t)(uint8_t)buf[offset + 7] << 56; + k1 ^= tail_byte(buf, offset + 7, signed_tail) << 56; // fallthrough case 7: - k1 ^= (uint64_t)(uint8_t)buf[offset + 6] << 48; + k1 ^= tail_byte(buf, offset + 6, signed_tail) << 48; // fallthrough case 6: - k1 ^= (uint64_t)(uint8_t)buf[offset + 5] << 40; + k1 ^= tail_byte(buf, offset + 5, signed_tail) << 40; // fallthrough case 5: - k1 ^= (uint64_t)(uint8_t)buf[offset + 4] << 32; + k1 ^= tail_byte(buf, offset + 4, signed_tail) << 32; // fallthrough case 4: - k1 ^= (uint64_t)(uint8_t)buf[offset + 3] << 24; + k1 ^= tail_byte(buf, offset + 3, signed_tail) << 24; // fallthrough case 3: - k1 ^= (uint64_t)(uint8_t)buf[offset + 2] << 16; + k1 ^= tail_byte(buf, offset + 2, signed_tail) << 16; // fallthrough case 2: - k1 ^= (uint64_t)(uint8_t)buf[offset + 1] << 8; + k1 ^= tail_byte(buf, offset + 1, signed_tail) << 8; // fallthrough case 1: - k1 ^= (uint64_t)(uint8_t)buf[offset]; + k1 ^= tail_byte(buf, offset, signed_tail); k1 *= c1; k1 = rotl64(k1, 31); k1 *= c2; diff --git a/cpp/src/common/container/murmur_hash3.h b/cpp/src/common/container/murmur_hash3.h index da89ac3ce6..ec5f3a5357 100644 --- a/cpp/src/common/container/murmur_hash3.h +++ b/cpp/src/common/container/murmur_hash3.h @@ -49,8 +49,20 @@ class Murmur128Hash { seed); } + // Java sign-extends the tail bytes (`(long) key[i]`); C++ historically + // zero-extended them. The two differ whenever a tail byte has its high bit + // set, i.e. for multi-byte UTF-8 in the path. Bloom filter positions are + // persisted in the file, so this entry point must match Java. FORCE_INLINE static int32_t hash(const common::String& buf, int32_t seed) { - return inner_hash(buf.buf_, buf.len_ - 1, seed); + return inner_hash(buf.buf_, buf.len_ - 1, seed, /*signed_tail=*/true); + } + + // Zero-extends tail bytes, as C++ writers did before the Java-compatible + // behaviour above was adopted. Readers still have to accept bloom filters + // produced that way, since the bits are already on disk. + FORCE_INLINE static int32_t hash_legacy_unsigned(const common::String& buf, + int32_t seed) { + return inner_hash(buf.buf_, buf.len_ - 1, seed, /*signed_tail=*/false); } private: @@ -68,7 +80,8 @@ class Murmur128Hash { return (int64_t)uk; } static int64_t get_block(const char* buf, int32_t index); - static int64_t inner_hash(const char* buf, int32_t len, int64_t seed); + static int64_t inner_hash(const char* buf, int32_t len, int64_t seed, + bool signed_tail = false); }; } // end namespace common diff --git a/cpp/src/reader/bloom_filter.cc b/cpp/src/reader/bloom_filter.cc index 09e2c9a156..d99bc0fd92 100644 --- a/cpp/src/reader/bloom_filter.cc +++ b/cpp/src/reader/bloom_filter.cc @@ -217,15 +217,27 @@ bool BloomFilter::contains(const String& device_name, if (IS_NULL(entry.buf_)) { return true; // OOM — conservatively assume present } + // Filters on disk were built with either the Java (signed tail bytes) or + // the legacy C++ (zero-extended) variant, and the two only diverge when a + // tail byte is non-ASCII. Accept a hit under either variant; a bloom + // filter may return false positives, never false negatives. + bool present = true; + bool present_legacy_unsigned = true; for (uint32_t i = 0; i < hash_func_count_; i++) { - int32_t hv = hash_func_arr_[i].hash(entry); - if (!bitset_.get(hv)) { - free_entry_buf(entry.buf_); - return false; // definitely not present + HashFunction& hf = hash_func_arr_[i]; + if (present && !bitset_.get(hf.hash(entry))) { + present = false; + } + if (present_legacy_unsigned && + !bitset_.get(hf.hash_legacy_unsigned(entry))) { + present_legacy_unsigned = false; + } + if (!present && !present_legacy_unsigned) { + break; } } free_entry_buf(entry.buf_); - return true; // probably present + return present || present_legacy_unsigned; } int BloomFilter::serialize_to(ByteStream& out) { diff --git a/cpp/src/reader/bloom_filter.h b/cpp/src/reader/bloom_filter.h index 323cfa8a41..a2ab919540 100644 --- a/cpp/src/reader/bloom_filter.h +++ b/cpp/src/reader/bloom_filter.h @@ -35,7 +35,18 @@ class HashFunction { seed_ = seed; } FORCE_INLINE int32_t hash(common::String buf) { - int res = common::Murmur128Hash::hash(buf, seed_); + return hash_impl(buf, /*java_tail=*/true); + } + // Positions written by C++ versions that zero-extended tail bytes. + FORCE_INLINE int32_t hash_legacy_unsigned(common::String buf) { + return hash_impl(buf, /*java_tail=*/false); + } + + private: + FORCE_INLINE int32_t hash_impl(common::String buf, bool java_tail) { + int res = java_tail + ? common::Murmur128Hash::hash(buf, seed_) + : common::Murmur128Hash::hash_legacy_unsigned(buf, seed_); if (res == INT32_MIN) { res = 0; } @@ -43,7 +54,6 @@ class HashFunction { return res; } - private: int32_t cap_; int32_t seed_; }; @@ -117,6 +127,8 @@ class BloomFilter { int serialize_to(common::ByteStream& out); int deserialize_from(common::ByteStream& in); bool is_empty() const { return size_ == 0; } + uint32_t get_size() const { return size_; } + uint32_t get_hash_func_count() const { return hash_func_count_; } BitSet* get_bit_set() { return &bitset_; } private: diff --git a/cpp/test/reader/bloom_filter_test.cc b/cpp/test/reader/bloom_filter_test.cc index 46a43e4661..3c100cf135 100644 --- a/cpp/test/reader/bloom_filter_test.cc +++ b/cpp/test/reader/bloom_filter_test.cc @@ -113,3 +113,63 @@ TEST(BloomfilterTest, RejectsFilterLengthBeyondRemainingInput) { BloomFilter filter; EXPECT_EQ(common::E_TSFILE_CORRUPTED, filter.deserialize_from(out)); } + +TEST(BloomfilterTest, MurmurHashMatchesJavaForNonAsciiTail) { + // Expected values from org.apache.tsfile.utils.Murmur128Hash.hash(String, + // int). Java sign-extends tail bytes, which matters once the tail contains + // a multi-byte UTF-8 character. + // Path entries keep their trailing '\0' in len_ (hash() drops it). + char dorm_buf[] = "root.sensors.宿舍.TH.humidity"; + common::String dorm(dorm_buf, sizeof(dorm_buf)); + EXPECT_EQ(1456000382, common::Murmur128Hash::hash(dorm, 5)); + EXPECT_EQ(-210460151, common::Murmur128Hash::hash(dorm, 7)); + EXPECT_EQ(2133117767, common::Murmur128Hash::hash(dorm, 11)); + // The legacy C++ variant zero-extends and therefore lands elsewhere. + EXPECT_NE(common::Murmur128Hash::hash(dorm, 5), + common::Murmur128Hash::hash_legacy_unsigned(dorm, 5)); + + // ASCII tails are unaffected, so existing files keep their positions. + char ascii_buf[] = "test_table.test1.test.value"; + common::String ascii(ascii_buf, sizeof(ascii_buf)); + EXPECT_EQ(common::Murmur128Hash::hash(ascii, 5), + common::Murmur128Hash::hash_legacy_unsigned(ascii, 5)); +} + +TEST(BloomfilterTest, ContainsFindsNonAsciiPathAddedByCpp) { + BloomFilter filter; + ASSERT_EQ(common::E_OK, filter.init(0.1, 2)); + common::PageArena arena; + common::String device = common::String("root.sensors.宿舍.TH", arena); + common::String sensor = common::String("humidity", arena); + ASSERT_EQ(common::E_OK, filter.add_path_entry(device, sensor)); + EXPECT_TRUE(filter.contains(device, sensor)); + + common::String other_device = + common::String("root.sensors.实验1.TH", arena); + common::String other_sensor = common::String("temperature", arena); + EXPECT_FALSE(filter.contains(other_device, other_sensor)); +} + +TEST(BloomfilterTest, ContainsAcceptsLegacyCppPositions) { + BloomFilter filter; + ASSERT_EQ(common::E_OK, filter.init(0.1, 2)); + common::PageArena arena; + common::String device = common::String("root.sensors.宿舍.TH", arena); + common::String sensor = common::String("humidity", arena); + char entry_buf[] = "root.sensors.宿舍.TH.humidity"; + common::String entry(entry_buf, sizeof(entry_buf)); + + // Reproduce what a C++ writer did before it adopted Java's signed tail: + // only the zero-extended positions are set. + for (uint32_t i = 0; i < filter.get_hash_func_count(); i++) { + int32_t res = common::Murmur128Hash::hash_legacy_unsigned( + entry, BloomFilter::SEEDS[i]); + if (res == INT32_MIN) { + res = 0; + } + int32_t pos = (res > 0 ? res : (0 - res)) % + static_cast(filter.get_size()); + filter.get_bit_set()->set(pos); + } + EXPECT_TRUE(filter.contains(device, sensor)); +} From bb4a3de707f63748aeb859f4c6e97ebcfb133598 Mon Sep 17 00:00:00 2001 From: ColinLee Date: Mon, 9 Nov 2026 10:00:37 +0800 Subject: [PATCH 2/4] [C++] Drop legacy bloom hash fallback Bloom filter positions are now produced and read exclusively with Java semantics (sign-extended tail bytes). Reading the zero-extended positions that older C++ releases wrote is no longer supported, so files written by those releases with non-ASCII device paths are no longer readable. Refs #1077 --- cpp/src/common/container/murmur_hash3.cc | 46 +++++++++++------------- cpp/src/common/container/murmur_hash3.h | 20 +++-------- cpp/src/reader/bloom_filter.cc | 22 +++--------- cpp/src/reader/bloom_filter.h | 16 ++------- cpp/test/reader/bloom_filter_test.cc | 32 ++--------------- 5 files changed, 35 insertions(+), 101 deletions(-) diff --git a/cpp/src/common/container/murmur_hash3.cc b/cpp/src/common/container/murmur_hash3.cc index d29cb4ba99..e8e00d5db6 100644 --- a/cpp/src/common/container/murmur_hash3.cc +++ b/cpp/src/common/container/murmur_hash3.cc @@ -24,19 +24,15 @@ namespace common { // follow Java IoTDB exactly. namespace { // Java's byte is signed and its tail loop does `(long) key[i]`, so bytes with -// the high bit set contribute 0xFFFF...FF instead of 0x00...FF. -FORCE_INLINE uint64_t tail_byte(const char* buf, int32_t index, - bool signed_tail) { - if (signed_tail) { - return static_cast( - static_cast(static_cast(buf[index]))); - } - return static_cast(static_cast(buf[index])); +// the high bit set contribute 0xFFFF...FF instead of 0x00...FF. Bloom filter +// positions are persisted in the file and must match Java's. +FORCE_INLINE uint64_t tail_byte(const char* buf, int32_t index) { + return static_cast( + static_cast(static_cast(buf[index]))); } } // namespace -int64_t Murmur128Hash::inner_hash(const char* buf, int32_t len, int64_t seed, - bool signed_tail) { +int64_t Murmur128Hash::inner_hash(const char* buf, int32_t len, int64_t seed) { const int32_t block_count = len >> 4; uint64_t h1 = static_cast(seed); uint64_t h2 = static_cast(seed); @@ -71,53 +67,53 @@ int64_t Murmur128Hash::inner_hash(const char* buf, int32_t len, int64_t seed, uint64_t k2 = 0; switch (len & 15) { case 15: - k2 ^= tail_byte(buf, offset + 14, signed_tail) << 48; + k2 ^= tail_byte(buf, offset + 14) << 48; // fallthrough case 14: - k2 ^= tail_byte(buf, offset + 13, signed_tail) << 40; + k2 ^= tail_byte(buf, offset + 13) << 40; // fallthrough case 13: - k2 ^= tail_byte(buf, offset + 12, signed_tail) << 32; + k2 ^= tail_byte(buf, offset + 12) << 32; // fallthrough case 12: - k2 ^= tail_byte(buf, offset + 11, signed_tail) << 24; + k2 ^= tail_byte(buf, offset + 11) << 24; // fallthrough case 11: - k2 ^= tail_byte(buf, offset + 10, signed_tail) << 16; + k2 ^= tail_byte(buf, offset + 10) << 16; // fallthrough case 10: - k2 ^= tail_byte(buf, offset + 9, signed_tail) << 8; + k2 ^= tail_byte(buf, offset + 9) << 8; // fallthrough case 9: - k2 ^= tail_byte(buf, offset + 8, signed_tail); + k2 ^= tail_byte(buf, offset + 8); k2 *= c2; k2 = rotl64(k2, 33); k2 *= c1; h2 ^= k2; // fallthrough case 8: - k1 ^= tail_byte(buf, offset + 7, signed_tail) << 56; + k1 ^= tail_byte(buf, offset + 7) << 56; // fallthrough case 7: - k1 ^= tail_byte(buf, offset + 6, signed_tail) << 48; + k1 ^= tail_byte(buf, offset + 6) << 48; // fallthrough case 6: - k1 ^= tail_byte(buf, offset + 5, signed_tail) << 40; + k1 ^= tail_byte(buf, offset + 5) << 40; // fallthrough case 5: - k1 ^= tail_byte(buf, offset + 4, signed_tail) << 32; + k1 ^= tail_byte(buf, offset + 4) << 32; // fallthrough case 4: - k1 ^= tail_byte(buf, offset + 3, signed_tail) << 24; + k1 ^= tail_byte(buf, offset + 3) << 24; // fallthrough case 3: - k1 ^= tail_byte(buf, offset + 2, signed_tail) << 16; + k1 ^= tail_byte(buf, offset + 2) << 16; // fallthrough case 2: - k1 ^= tail_byte(buf, offset + 1, signed_tail) << 8; + k1 ^= tail_byte(buf, offset + 1) << 8; // fallthrough case 1: - k1 ^= tail_byte(buf, offset, signed_tail); + k1 ^= tail_byte(buf, offset); k1 *= c1; k1 = rotl64(k1, 31); k1 *= c2; diff --git a/cpp/src/common/container/murmur_hash3.h b/cpp/src/common/container/murmur_hash3.h index ec5f3a5357..740e1435dd 100644 --- a/cpp/src/common/container/murmur_hash3.h +++ b/cpp/src/common/container/murmur_hash3.h @@ -49,20 +49,11 @@ class Murmur128Hash { seed); } - // Java sign-extends the tail bytes (`(long) key[i]`); C++ historically - // zero-extended them. The two differ whenever a tail byte has its high bit - // set, i.e. for multi-byte UTF-8 in the path. Bloom filter positions are - // persisted in the file, so this entry point must match Java. + // Java sign-extends the tail bytes (`(long) key[i]`). Bloom filter + // positions are persisted in the file, so C++ has to sign-extend too or a + // non-ASCII path yields positions Java never set. FORCE_INLINE static int32_t hash(const common::String& buf, int32_t seed) { - return inner_hash(buf.buf_, buf.len_ - 1, seed, /*signed_tail=*/true); - } - - // Zero-extends tail bytes, as C++ writers did before the Java-compatible - // behaviour above was adopted. Readers still have to accept bloom filters - // produced that way, since the bits are already on disk. - FORCE_INLINE static int32_t hash_legacy_unsigned(const common::String& buf, - int32_t seed) { - return inner_hash(buf.buf_, buf.len_ - 1, seed, /*signed_tail=*/false); + return inner_hash(buf.buf_, buf.len_ - 1, seed); } private: @@ -80,8 +71,7 @@ class Murmur128Hash { return (int64_t)uk; } static int64_t get_block(const char* buf, int32_t index); - static int64_t inner_hash(const char* buf, int32_t len, int64_t seed, - bool signed_tail = false); + static int64_t inner_hash(const char* buf, int32_t len, int64_t seed); }; } // end namespace common diff --git a/cpp/src/reader/bloom_filter.cc b/cpp/src/reader/bloom_filter.cc index d99bc0fd92..09e2c9a156 100644 --- a/cpp/src/reader/bloom_filter.cc +++ b/cpp/src/reader/bloom_filter.cc @@ -217,27 +217,15 @@ bool BloomFilter::contains(const String& device_name, if (IS_NULL(entry.buf_)) { return true; // OOM — conservatively assume present } - // Filters on disk were built with either the Java (signed tail bytes) or - // the legacy C++ (zero-extended) variant, and the two only diverge when a - // tail byte is non-ASCII. Accept a hit under either variant; a bloom - // filter may return false positives, never false negatives. - bool present = true; - bool present_legacy_unsigned = true; for (uint32_t i = 0; i < hash_func_count_; i++) { - HashFunction& hf = hash_func_arr_[i]; - if (present && !bitset_.get(hf.hash(entry))) { - present = false; - } - if (present_legacy_unsigned && - !bitset_.get(hf.hash_legacy_unsigned(entry))) { - present_legacy_unsigned = false; - } - if (!present && !present_legacy_unsigned) { - break; + int32_t hv = hash_func_arr_[i].hash(entry); + if (!bitset_.get(hv)) { + free_entry_buf(entry.buf_); + return false; // definitely not present } } free_entry_buf(entry.buf_); - return present || present_legacy_unsigned; + return true; // probably present } int BloomFilter::serialize_to(ByteStream& out) { diff --git a/cpp/src/reader/bloom_filter.h b/cpp/src/reader/bloom_filter.h index a2ab919540..323cfa8a41 100644 --- a/cpp/src/reader/bloom_filter.h +++ b/cpp/src/reader/bloom_filter.h @@ -35,18 +35,7 @@ class HashFunction { seed_ = seed; } FORCE_INLINE int32_t hash(common::String buf) { - return hash_impl(buf, /*java_tail=*/true); - } - // Positions written by C++ versions that zero-extended tail bytes. - FORCE_INLINE int32_t hash_legacy_unsigned(common::String buf) { - return hash_impl(buf, /*java_tail=*/false); - } - - private: - FORCE_INLINE int32_t hash_impl(common::String buf, bool java_tail) { - int res = java_tail - ? common::Murmur128Hash::hash(buf, seed_) - : common::Murmur128Hash::hash_legacy_unsigned(buf, seed_); + int res = common::Murmur128Hash::hash(buf, seed_); if (res == INT32_MIN) { res = 0; } @@ -54,6 +43,7 @@ class HashFunction { return res; } + private: int32_t cap_; int32_t seed_; }; @@ -127,8 +117,6 @@ class BloomFilter { int serialize_to(common::ByteStream& out); int deserialize_from(common::ByteStream& in); bool is_empty() const { return size_ == 0; } - uint32_t get_size() const { return size_; } - uint32_t get_hash_func_count() const { return hash_func_count_; } BitSet* get_bit_set() { return &bitset_; } private: diff --git a/cpp/test/reader/bloom_filter_test.cc b/cpp/test/reader/bloom_filter_test.cc index 3c100cf135..00e3bb62bb 100644 --- a/cpp/test/reader/bloom_filter_test.cc +++ b/cpp/test/reader/bloom_filter_test.cc @@ -124,15 +124,11 @@ TEST(BloomfilterTest, MurmurHashMatchesJavaForNonAsciiTail) { EXPECT_EQ(1456000382, common::Murmur128Hash::hash(dorm, 5)); EXPECT_EQ(-210460151, common::Murmur128Hash::hash(dorm, 7)); EXPECT_EQ(2133117767, common::Murmur128Hash::hash(dorm, 11)); - // The legacy C++ variant zero-extends and therefore lands elsewhere. - EXPECT_NE(common::Murmur128Hash::hash(dorm, 5), - common::Murmur128Hash::hash_legacy_unsigned(dorm, 5)); - // ASCII tails are unaffected, so existing files keep their positions. + // ASCII tails are unaffected. char ascii_buf[] = "test_table.test1.test.value"; common::String ascii(ascii_buf, sizeof(ascii_buf)); - EXPECT_EQ(common::Murmur128Hash::hash(ascii, 5), - common::Murmur128Hash::hash_legacy_unsigned(ascii, 5)); + EXPECT_EQ(-823637368, common::Murmur128Hash::hash(ascii, 5)); } TEST(BloomfilterTest, ContainsFindsNonAsciiPathAddedByCpp) { @@ -149,27 +145,3 @@ TEST(BloomfilterTest, ContainsFindsNonAsciiPathAddedByCpp) { common::String other_sensor = common::String("temperature", arena); EXPECT_FALSE(filter.contains(other_device, other_sensor)); } - -TEST(BloomfilterTest, ContainsAcceptsLegacyCppPositions) { - BloomFilter filter; - ASSERT_EQ(common::E_OK, filter.init(0.1, 2)); - common::PageArena arena; - common::String device = common::String("root.sensors.宿舍.TH", arena); - common::String sensor = common::String("humidity", arena); - char entry_buf[] = "root.sensors.宿舍.TH.humidity"; - common::String entry(entry_buf, sizeof(entry_buf)); - - // Reproduce what a C++ writer did before it adopted Java's signed tail: - // only the zero-extended positions are set. - for (uint32_t i = 0; i < filter.get_hash_func_count(); i++) { - int32_t res = common::Murmur128Hash::hash_legacy_unsigned( - entry, BloomFilter::SEEDS[i]); - if (res == INT32_MIN) { - res = 0; - } - int32_t pos = (res > 0 ? res : (0 - res)) % - static_cast(filter.get_size()); - filter.get_bit_set()->set(pos); - } - EXPECT_TRUE(filter.contains(device, sensor)); -} From e612cb93d67c8b81e1ae0b095ffaabd57fbb1ab2 Mon Sep 17 00:00:00 2001 From: ColinLee Date: Mon, 9 Nov 2026 11:57:01 +0800 Subject: [PATCH 3/4] [C++][Java] Cover non-ASCII device and measurement names in tests Add a tree-model round trip using multi-byte UTF-8 device and measurement names on both implementations, and a Java-written fixture read by the C++ reader. The fixture guards the cross-language bloom filter positions: the C++ reader used to drop both series and now returns all rows. Refs #1077 --- .../reader/fixtures/non_ascii_tree.tsfile | Bin 0 -> 487 bytes cpp/test/reader/non_ascii_path_test.cc | 118 ++++++++++++++++++ .../apache/tsfile/read/NonAsciiPathTest.java | 100 +++++++++++++++ 3 files changed, 218 insertions(+) create mode 100644 cpp/test/reader/fixtures/non_ascii_tree.tsfile create mode 100644 cpp/test/reader/non_ascii_path_test.cc create mode 100644 java/tsfile/src/test/java/org/apache/tsfile/read/NonAsciiPathTest.java diff --git a/cpp/test/reader/fixtures/non_ascii_tree.tsfile b/cpp/test/reader/fixtures/non_ascii_tree.tsfile new file mode 100644 index 0000000000000000000000000000000000000000..c1e2ec14133534a304c2bbbdb53f406fd7fa6ea2 GIT binary patch literal 487 zcmWG3cFW93Wno|vE6UF=(R;SF`^EaHd@t7Rd%C3Ekd^1zj+IY$EmLJ-=g?P9lVD(A z;ACKA5MtzJaAJPpV8X@l;5_pje#^!`hKFq|-2Qg~HSF9E)SwR3V4#Yj!IO*O1H%R8 zIpRH!XUQLyv2d@?W?*7~TEfD?1vDQBm|sBYj}9gfmV+&nM(5`(fJi_{S)fs1>#-T- z2{ppE0nBhf=Qnad6hKHnMyRWxf=v*bQG%HfIiMJ^1{+K*ls0698p6;htHi*<1mXz* zaRZ0}1p*8L4jc_ktZWjgdC5hU1tmVIWvMw#1|m?dcWNbr7?d4SS&-_LUzDzwSdf^U ik*ZfxoCb_SJ*cc+sGqB!b5LYJh^q_G@HHXe2nGPSyLU + +#include +#include +#include +#include + +#include "common/schema.h" +#include "common/tablet.h" +#include "reader/tsfile_reader.h" +#include "writer/tsfile_writer.h" + +using namespace storage; + +namespace { + +// Multi-byte UTF-8 in the device and measurement names reaches the bloom +// filter tail, where Java and C++ used to disagree on byte sign extension. +const char* const kDevice = "root.测试.设备1"; +const char* const kTemperature = "温度"; +const char* const kHumidity = "湿度"; +const char* const kRoundTripFile = "non_ascii_path_roundtrip.tsfile"; + +// Returns the number of rows, or -1 when the query itself failed. +int64_t CountRows(TsFileReader& reader, const std::vector& paths) { + std::vector query_paths(paths); + ResultSet* result = nullptr; + if (reader.query(query_paths, INT64_MIN, INT64_MAX, result) != + common::E_OK || + result == nullptr) { + return -1; + } + int64_t rows = 0; + bool has_next = false; + while (result->next(has_next) == common::E_OK && has_next) { + ++rows; + } + reader.destroy_query_data_set(result); + return rows; +} + +} // namespace + +// The fixture was written by the Java implementation (TsFile 2.5.0) with: +// device root.测试.设备1 +// measurements 温度, 湿度 (DOUBLE, GORILLA, LZ4) +// rows 10, timestamps 1000..1009, values 20+i and 50+i +// The reader must not lose a series just because the path is non-ASCII. +TEST(NonAsciiPathTest, ReadsJavaWrittenNonAsciiPaths) { + libtsfile_init(); + const std::string fixture = std::string(TSFILE_CPP_SOURCE_DIR) + + "/test/reader/fixtures/non_ascii_tree.tsfile"; + TsFileReader reader; + ASSERT_EQ(common::E_OK, reader.open(fixture)); + + const std::string device(kDevice); + EXPECT_EQ(10, CountRows(reader, {device + "." + kTemperature})); + EXPECT_EQ(10, CountRows(reader, {device + "." + kHumidity})); + EXPECT_EQ(10, CountRows( + reader, {device + "." + kTemperature, + device + "." + kHumidity})); + reader.close(); +} + +TEST(NonAsciiPathTest, RoundTripsNonAsciiPaths) { + libtsfile_init(); + std::remove(kRoundTripFile); + { + TsFileWriter writer; + ASSERT_EQ(common::E_OK, writer.open(kRoundTripFile)); + MeasurementSchema temperature(kTemperature, common::DOUBLE, + common::GORILLA, common::LZ4); + MeasurementSchema humidity(kHumidity, common::DOUBLE, common::GORILLA, + common::LZ4); + ASSERT_EQ(common::E_OK, + writer.register_timeseries(kDevice, temperature)); + ASSERT_EQ(common::E_OK, writer.register_timeseries(kDevice, humidity)); + + auto schemas = std::make_shared>(); + schemas->push_back(temperature); + schemas->push_back(humidity); + Tablet tablet(kDevice, schemas); + for (int i = 0; i < 10; i++) { + tablet.add_timestamp(static_cast(i), 1000 + i); + tablet.add_value(static_cast(i), 0U, 20.0 + i); + tablet.add_value(static_cast(i), 1U, 50.0 + i); + } + ASSERT_EQ(common::E_OK, writer.write_tablet(tablet)); + writer.close(); + } + { + TsFileReader reader; + ASSERT_EQ(common::E_OK, reader.open(kRoundTripFile)); + const std::string device(kDevice); + EXPECT_EQ(10, CountRows(reader, {device + "." + kTemperature, + device + "." + kHumidity})); + reader.close(); + } + std::remove(kRoundTripFile); +} diff --git a/java/tsfile/src/test/java/org/apache/tsfile/read/NonAsciiPathTest.java b/java/tsfile/src/test/java/org/apache/tsfile/read/NonAsciiPathTest.java new file mode 100644 index 0000000000..845fdea14d --- /dev/null +++ b/java/tsfile/src/test/java/org/apache/tsfile/read/NonAsciiPathTest.java @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tsfile.read; + +import org.apache.tsfile.enums.TSDataType; +import org.apache.tsfile.file.metadata.enums.CompressionType; +import org.apache.tsfile.file.metadata.enums.TSEncoding; +import org.apache.tsfile.read.common.Field; +import org.apache.tsfile.read.common.Path; +import org.apache.tsfile.read.common.RowRecord; +import org.apache.tsfile.read.expression.QueryExpression; +import org.apache.tsfile.read.query.dataset.QueryDataSet; +import org.apache.tsfile.write.TsFileWriter; +import org.apache.tsfile.write.record.Tablet; +import org.apache.tsfile.write.schema.IMeasurementSchema; +import org.apache.tsfile.write.schema.MeasurementSchema; + +import org.junit.Assert; +import org.junit.Test; + +import java.io.File; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Device and measurement names containing multi-byte UTF-8 characters take part in the persisted + * bloom filter positions, so they need explicit coverage. + */ +public class NonAsciiPathTest { + + private static final String DEVICE = "root.测试.设备1"; + private static final String TEMPERATURE = "温度"; + private static final String HUMIDITY = "湿度"; + private static final int ROWS = 20; + + @Test + public void treeModelReadWrite() throws Exception { + File file = File.createTempFile("non_ascii_tree", ".tsfile"); + Files.deleteIfExists(file.toPath()); + List schemas = + Arrays.asList( + new MeasurementSchema( + TEMPERATURE, TSDataType.DOUBLE, TSEncoding.GORILLA, CompressionType.LZ4), + new MeasurementSchema( + HUMIDITY, TSDataType.DOUBLE, TSEncoding.GORILLA, CompressionType.LZ4)); + try { + try (TsFileWriter writer = new TsFileWriter(file)) { + writer.registerTimeseries(new Path(DEVICE), schemas); + Tablet tablet = new Tablet(DEVICE, schemas); + for (int i = 0; i < ROWS; i++) { + tablet.addTimestamp(i, 1000L + i); + tablet.addValue(i, 0, 20.0 + i); + tablet.addValue(i, 1, 50.0 + i); + } + writer.writeTree(tablet); + } + + try (TsFileSequenceReader sequenceReader = new TsFileSequenceReader(file.getPath()); + TsFileReader reader = new TsFileReader(sequenceReader)) { + Assert.assertEquals(1, sequenceReader.getAllDevices().size()); + + List paths = new ArrayList<>(); + paths.add(new Path(DEVICE, TEMPERATURE, true)); + paths.add(new Path(DEVICE, HUMIDITY, true)); + try (QueryDataSet dataSet = reader.query(QueryExpression.create(paths, null))) { + int rowIndex = 0; + while (dataSet.hasNext()) { + RowRecord row = dataSet.next(); + List fields = row.getFields(); + Assert.assertEquals(1000L + rowIndex, row.getTimestamp()); + Assert.assertEquals(20.0 + rowIndex, fields.get(0).getDoubleV(), 1e-9); + Assert.assertEquals(50.0 + rowIndex, fields.get(1).getDoubleV(), 1e-9); + rowIndex++; + } + Assert.assertEquals(ROWS, rowIndex); + } + } + } finally { + Files.deleteIfExists(file.toPath()); + } + } +} From 62000ed44f3fa1c668a0a3b75c1b73c10edb2466 Mon Sep 17 00:00:00 2001 From: ColinLee Date: Mon, 9 Nov 2026 12:00:26 +0800 Subject: [PATCH 4/4] [C++] Fix spotless formatting in non-ASCII path test Refs #1077 --- cpp/test/reader/non_ascii_path_test.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp/test/reader/non_ascii_path_test.cc b/cpp/test/reader/non_ascii_path_test.cc index f3dcb8cb2a..fe5fc4b1bc 100644 --- a/cpp/test/reader/non_ascii_path_test.cc +++ b/cpp/test/reader/non_ascii_path_test.cc @@ -74,9 +74,8 @@ TEST(NonAsciiPathTest, ReadsJavaWrittenNonAsciiPaths) { const std::string device(kDevice); EXPECT_EQ(10, CountRows(reader, {device + "." + kTemperature})); EXPECT_EQ(10, CountRows(reader, {device + "." + kHumidity})); - EXPECT_EQ(10, CountRows( - reader, {device + "." + kTemperature, - device + "." + kHumidity})); + EXPECT_EQ(10, CountRows(reader, {device + "." + kTemperature, + device + "." + kHumidity})); reader.close(); }