diff --git a/cpp/src/common/container/murmur_hash3.cc b/cpp/src/common/container/murmur_hash3.cc index 256542333..e8e00d5db 100644 --- a/cpp/src/common/container/murmur_hash3.cc +++ b/cpp/src/common/container/murmur_hash3.cc @@ -22,6 +22,16 @@ namespace common { /* ================ Murmur128Hash ================ */ // 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. 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) { const int32_t block_count = len >> 4; uint64_t h1 = static_cast(seed); @@ -57,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 ^= (uint64_t)(uint8_t)buf[offset + 14] << 48; + k2 ^= tail_byte(buf, offset + 14) << 48; // fallthrough case 14: - k2 ^= (uint64_t)(uint8_t)buf[offset + 13] << 40; + k2 ^= tail_byte(buf, offset + 13) << 40; // fallthrough case 13: - k2 ^= (uint64_t)(uint8_t)buf[offset + 12] << 32; + k2 ^= tail_byte(buf, offset + 12) << 32; // fallthrough case 12: - k2 ^= (uint64_t)(uint8_t)buf[offset + 11] << 24; + k2 ^= tail_byte(buf, offset + 11) << 24; // fallthrough case 11: - k2 ^= (uint64_t)(uint8_t)buf[offset + 10] << 16; + k2 ^= tail_byte(buf, offset + 10) << 16; // fallthrough case 10: - k2 ^= (uint64_t)(uint8_t)buf[offset + 9] << 8; + k2 ^= tail_byte(buf, offset + 9) << 8; // fallthrough case 9: - k2 ^= (uint64_t)(uint8_t)buf[offset + 8]; + k2 ^= tail_byte(buf, offset + 8); 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) << 56; // fallthrough case 7: - k1 ^= (uint64_t)(uint8_t)buf[offset + 6] << 48; + k1 ^= tail_byte(buf, offset + 6) << 48; // fallthrough case 6: - k1 ^= (uint64_t)(uint8_t)buf[offset + 5] << 40; + k1 ^= tail_byte(buf, offset + 5) << 40; // fallthrough case 5: - k1 ^= (uint64_t)(uint8_t)buf[offset + 4] << 32; + k1 ^= tail_byte(buf, offset + 4) << 32; // fallthrough case 4: - k1 ^= (uint64_t)(uint8_t)buf[offset + 3] << 24; + k1 ^= tail_byte(buf, offset + 3) << 24; // fallthrough case 3: - k1 ^= (uint64_t)(uint8_t)buf[offset + 2] << 16; + k1 ^= tail_byte(buf, offset + 2) << 16; // fallthrough case 2: - k1 ^= (uint64_t)(uint8_t)buf[offset + 1] << 8; + k1 ^= tail_byte(buf, offset + 1) << 8; // fallthrough case 1: - k1 ^= (uint64_t)(uint8_t)buf[offset]; + 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 da89ac3ce..740e1435d 100644 --- a/cpp/src/common/container/murmur_hash3.h +++ b/cpp/src/common/container/murmur_hash3.h @@ -49,6 +49,9 @@ class Murmur128Hash { seed); } + // 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); } diff --git a/cpp/test/reader/bloom_filter_test.cc b/cpp/test/reader/bloom_filter_test.cc index 46a43e466..00e3bb62b 100644 --- a/cpp/test/reader/bloom_filter_test.cc +++ b/cpp/test/reader/bloom_filter_test.cc @@ -113,3 +113,35 @@ 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)); + + // ASCII tails are unaffected. + char ascii_buf[] = "test_table.test1.test.value"; + common::String ascii(ascii_buf, sizeof(ascii_buf)); + EXPECT_EQ(-823637368, common::Murmur128Hash::hash(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)); +} 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 000000000..c1e2ec141 Binary files /dev/null and b/cpp/test/reader/fixtures/non_ascii_tree.tsfile differ diff --git a/cpp/test/reader/non_ascii_path_test.cc b/cpp/test/reader/non_ascii_path_test.cc new file mode 100644 index 000000000..fe5fc4b1b --- /dev/null +++ b/cpp/test/reader/non_ascii_path_test.cc @@ -0,0 +1,117 @@ +/* + * 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. + */ +#include + +#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 000000000..845fdea14 --- /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()); + } + } +}