Repository navigation
[C++] Match Java bloom filter hash for non-ASCII device paths - #1079
Merged
Merged
Conversation
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 apache#1077
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 apache#1077
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 apache#1077
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Murmur128Hashdisagrees between Java and C++ for non-ASCII paths, so the C++/Python reader can silently drop a whole time series. Fixes #1077.Root cause
Java (
Murmur128Hash.innerHash) hashes tail bytes with((long) key[i]), which is signed and sign-extends bytes >= 0x80. C++ (murmur_hash3.cc) used(uint64_t)(uint8_t), which zero-extends. The results only diverge whendevice + "." + measurementhas a byte >= 0x80 in the tail (len % 16), i.e. exactly for multi-byte UTF-8 device path segments.Bloom filter positions are persisted in the file. Reading a Java-written file, C++ computed positions that were never set,
BloomFilter::contains()returned false andTsFileIOReader::alloc_ssi()returnedE_NO_MORE_DATA: the series was dropped without an error, and the query returned an all-NULL column.Fix
Hash bloom path entries exclusively with Java semantics (sign-extend tail bytes) on both the write and read paths, matching
Murmur128Hash.innerHash.No backward-compatibility shim is kept: files written by older C++ releases with non-ASCII device paths are no longer readable by this reader.
Tests
cpp/test/reader/non_ascii_path_test.ccReadsJavaWrittenNonAsciiPaths: readsfixtures/non_ascii_tree.tsfile, written by the Java implementation with deviceroot.测试.设备1and measurements温度/湿度. Before the fix both series were dropped (0 rows); now all rows are returned.RoundTripsNonAsciiPaths: C++ writer + reader round trip with the same non-ASCII names.java/tsfile/src/test/java/org/apache/tsfile/read/NonAsciiPathTest.java: tree-model write/read with a non-ASCII device and non-ASCII measurement names.cpp/test/reader/bloom_filter_test.cc: Java reference hash values for a non-ASCII path, and a non-ASCII add/contains round trip.Verification
NonAsciiPathTest(Java): passed.root.sensors.宿舍.TH:humidityreturns 402,412 rows (previously an all-NULL column),temperatureunchanged; ASCII device paths unaffected.