From 5d028c4a36e51cde7b17d6ae6d3d081956b69bc4 Mon Sep 17 00:00:00 2001 From: gx Date: Fri, 7 Aug 2026 17:27:51 +0800 Subject: [PATCH 01/13] fix(cpp): handle TS2DIFF float prefixes in batch decode --- cpp/src/encoding/ts2diff_decoder.h | 26 +++++---------- cpp/test/encoding/ts2diff_codec_test.cc | 43 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/cpp/src/encoding/ts2diff_decoder.h b/cpp/src/encoding/ts2diff_decoder.h index 206b7f559..f6fc961b0 100644 --- a/cpp/src/encoding/ts2diff_decoder.h +++ b/cpp/src/encoding/ts2diff_decoder.h @@ -952,15 +952,10 @@ class FloatTS2DIFFDecoder : public TS2DIFFDecoder { int read_batch_float(float* out, int capacity, int& actual, common::ByteStream& in) override { - // Reuse SIMD batch decode for int32, then bit-cast to float - int32_t* buf = reinterpret_cast(out); - int ret = TS2DIFFDecoder::read_batch_int32(buf, capacity, - actual, in); - if (ret != common::E_OK) return ret; - for (int i = 0; i < actual; ++i) { - out[i] = common::int_to_float(buf[i]); - } - return common::E_OK; + // FLOAT TS_2DIFF segments have a scale/overflow prefix before the + // integer delta block. The integer batch decoder does not consume + // that prefix, so use the segment-aware scalar decoder here. + return Decoder::read_batch_float(out, capacity, actual, in); } private: @@ -989,15 +984,10 @@ class DoubleTS2DIFFDecoder : public TS2DIFFDecoder { int read_batch_double(double* out, int capacity, int& actual, common::ByteStream& in) override { - // Reuse SIMD batch decode for int64, then bit-cast to double - int64_t* buf = reinterpret_cast(out); - int ret = TS2DIFFDecoder::read_batch_int64(buf, capacity, - actual, in); - if (ret != common::E_OK) return ret; - for (int i = 0; i < actual; ++i) { - out[i] = common::long_to_double(buf[i]); - } - return common::E_OK; + // DOUBLE TS_2DIFF uses the same segment prefix. Bypassing + // read_double() misreads that prefix as a block header and can spin + // at end-of-input while decoding an otherwise valid page. + return Decoder::read_batch_double(out, capacity, actual, in); } private: diff --git a/cpp/test/encoding/ts2diff_codec_test.cc b/cpp/test/encoding/ts2diff_codec_test.cc index fb997103c..43c86adef 100644 --- a/cpp/test/encoding/ts2diff_codec_test.cc +++ b/cpp/test/encoding/ts2diff_codec_test.cc @@ -187,6 +187,49 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, TestDoubleRoundTrip) { EXPECT_FALSE(decoder_double_->has_remaining(out_stream)); } +TEST_F(FloatDoubleTS2DIFFCodecTest, + ReadBatchFloatConsumesPrefixesAcrossSegments) { + common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 300; + std::vector expected(row_num); + for (int i = 0; i < row_num; ++i) { + expected[i] = static_cast(i) * 0.25f + 0.5f; + ASSERT_EQ(encoder_float_->encode(expected[i], out_stream), + common::E_OK); + } + ASSERT_EQ(encoder_float_->flush(out_stream), common::E_OK); + + std::vector actual_values(row_num); + int actual = 0; + ASSERT_EQ(decoder_float_->read_batch_float(actual_values.data(), row_num, + actual, out_stream), + common::E_OK); + ASSERT_EQ(actual, row_num); + for (int i = 0; i < row_num; ++i) { + EXPECT_FLOAT_EQ(actual_values[i], expected[i]) << "row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); +} + +TEST_F(FloatDoubleTS2DIFFCodecTest, ReadBatchDoubleConsumesOverflowPrefix) { + common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); + const double expected[] = {3.123456768E20, std::nan("")}; + for (double value : expected) { + ASSERT_EQ(encoder_double_->encode(value, out_stream), common::E_OK); + } + ASSERT_EQ(encoder_double_->flush(out_stream), common::E_OK); + + double actual_values[2] = {}; + int actual = 0; + ASSERT_EQ(decoder_double_->read_batch_double(actual_values, 2, actual, + out_stream), + common::E_OK); + ASSERT_EQ(actual, 2); + EXPECT_DOUBLE_EQ(actual_values[0], expected[0]); + EXPECT_TRUE(std::isnan(actual_values[1])); + EXPECT_FALSE(decoder_double_->has_remaining(out_stream)); +} + TEST_F(TS2DIFFCodecTest, TestIntEncoding1) { common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); const int row_num = 10000; From fca8dc526c3b241c11d6a08913e61c9edd3de25f Mon Sep 17 00:00:00 2001 From: gx Date: Tue, 18 Aug 2026 21:24:38 +0800 Subject: [PATCH 02/13] fix(cpp): make TS2DIFF float/double prefix detection unambiguous The per-block heuristic that distinguished Java-compatible maxPointNumber prefixes from legacy raw delta blocks could misclassify a valid raw header (wi = 0 or bit_width = 0 blocks), which desynced the stream and could spin at end-of-input in batch reads. Decide the page layout once per page instead: parse the whole remaining stream with the Java segment grammar (prefix + overflow bitmaps + block run, validated field ranges and exact exhaustion) and cache the segment prefix offsets. A legacy raw page fails this parse because its first misaligned write_index probe reads >= 0x100. - Legacy raw pages keep the integer SIMD batch decode path with bit-cast semantics (parent-commit behavior). - Java pages consume prefixes only at recorded offsets and take the segment-aware scalar path; this also fixes value semantics across blocks inside one Java segment, which the per-block heuristic could not represent. - Bail out of read_long() when the stream is exhausted with bits still owed, so no residual misconfiguration can loop forever. Also fix ByteStream::check_space(): after set_read_pos() parks the cursor at a page boundary, blindly following read_page_->next_ skipped the boundary page and failed reads with E_OUT_OF_RANGE. Recompute the page from the head instead; page chains are short so the walk is cheap. Add legacy raw batch/scalar/mixed regression tests for FLOAT and DOUBLE (PR #901 review). --- cpp/src/common/allocator/byte_stream.h | 13 +- cpp/src/encoding/ts2diff_decoder.h | 299 +++++++++++++++++++----- cpp/test/encoding/ts2diff_codec_test.cc | 157 +++++++++++++ 3 files changed, 411 insertions(+), 58 deletions(-) diff --git a/cpp/src/common/allocator/byte_stream.h b/cpp/src/common/allocator/byte_stream.h index 15f15b798..36933a8fd 100644 --- a/cpp/src/common/allocator/byte_stream.h +++ b/cpp/src/common/allocator/byte_stream.h @@ -696,7 +696,18 @@ class ByteStream { if (UNLIKELY(read_page_ == nullptr)) { read_page_ = head_.load(); } else if (UNLIKELY((read_pos_ & page_mask_) == 0)) { - read_page_ = read_page_->next_.load(); + // At a page boundary the cursor may have been parked here by a + // preceding sequential read (read_page_ is the page just + // finished, advance one) or by set_read_pos() (read_page_ is + // already the boundary page, advancing would skip it). The + // two states are indistinguishable, so recompute the page + // from the head instead of blindly following next_. + Page* p = head_.load(); + uint64_t page_idx = read_pos_ / page_size_; + while (p != nullptr && page_idx-- > 0) { + p = p->next_.load(); + } + read_page_ = p; } if (UNLIKELY(read_page_ == nullptr)) { return common::E_OUT_OF_RANGE; diff --git a/cpp/src/encoding/ts2diff_decoder.h b/cpp/src/encoding/ts2diff_decoder.h index f6fc961b0..a3d8f1282 100644 --- a/cpp/src/encoding/ts2diff_decoder.h +++ b/cpp/src/encoding/ts2diff_decoder.h @@ -219,37 +219,135 @@ inline bool bitmap_marked(const std::vector& bm, int idx) { return (bm[byte_idx] & static_cast(1u << (idx % 8))) != 0; } -inline bool looks_like_ts2diff_header(common::ByteStream& in) { - int ret = common::E_OK; - uint64_t probe_mark = in.read_pos(); - int32_t write_index = 0; - int32_t bit_width = 0; - if (RET_FAIL(common::SerializationUtil::read_i32(write_index, in)) || - RET_FAIL(common::SerializationUtil::read_i32(bit_width, in))) { - in.set_read_pos(probe_mark); - return false; - } - in.set_read_pos(probe_mark); - if (write_index < 0 || write_index > 128) { - return false; - } - if (bit_width < 0 || bit_width > 64) { - return false; +// Parse the remaining stream as one or more Java-compatible FLOAT/DOUBLE +// TS_2DIFF segments, recording the offset of every segment prefix. A +// segment is: +// [overflow flag][value count][underflow bitmap][overflow bitmap?] +// [maxPointNumber varint] block+ +// where a block is [write_index i32][bit_width i32][delta_min][first_value] +// followed by ceil(write_index*bit_width/8) packed bytes. The C++ encoder +// emits one segment per 128-value block, while the Java encoder emits one +// segment wrapping several consecutive blocks, hence "block+". +// +// A legacy raw page (plain delta blocks with no prefix at all) fails this +// parse in practice: its first write_index is >= 1 for any block produced +// by a real encoder, so after the varint tag eats the leading 0x00 byte +// the misaligned write_index probe reads >= 0x100 and is rejected. Only a +// byte-level coincidence could satisfy both interpretations. +// +// Returns true when the whole remaining stream is consumed exactly by the +// segment grammar; the read position is always restored. +inline bool scan_java_float_double_page(common::ByteStream& in, int value_bytes, + std::vector& prefix_offsets) { + const uint64_t page_start = in.read_pos(); + const int bw_limit = (value_bytes == 4) ? 32 : 64; + const int dmfv_bytes = value_bytes * 2; + prefix_offsets.clear(); + + auto skip_bytes = [&in](uint64_t n) -> bool { + uint8_t sink[64]; + while (n > 0) { + uint32_t chunk = n < sizeof(sink) + ? static_cast(n) + : static_cast(sizeof(sink)); + uint32_t got = 0; + if (in.read_buf(sink, chunk, got) != common::E_OK || got != chunk) { + return false; + } + n -= chunk; + } + return true; + }; + + bool valid = true; + while (valid && in.has_remaining()) { + prefix_offsets.push_back(in.read_pos()); + uint64_t group_value_count = 0; // sum of (write_index+1) per block + uint64_t expected_count = 0; // bitmap value count, 0 = no bitmap + uint32_t tag = 0; + if (common::SerializationUtil::read_var_uint(tag, in) != common::E_OK) { + valid = false; + break; + } + if (tag == FLAG_ORIGINAL_VALUE_OVERFLOW || + tag == FLAG_SCALED_VALUE_OVERFLOW) { + uint32_t n = 0; + if (common::SerializationUtil::read_var_uint(n, in) != + common::E_OK) { + valid = false; + break; + } + expected_count = n; + const uint64_t bm_len = static_cast(n) / 8 + 1; + if (!skip_bytes(bm_len)) { // underflow bitmap + valid = false; + break; + } + if (tag == FLAG_ORIGINAL_VALUE_OVERFLOW && !skip_bytes(bm_len)) { + valid = false; // overflow bitmap + break; + } + uint32_t mpn = 0; + if (common::SerializationUtil::read_var_uint(mpn, in) != + common::E_OK) { + valid = false; + break; + } + } + // Consume the blocks owned by this segment. A block run continues + // while a block header validates; an invalid header marks either + // the next segment prefix or a corrupt page (settled by whether + // the whole-stream parse consumes exactly below). + int blocks_in_segment = 0; + while (true) { + const uint64_t block_mark = in.read_pos(); + int32_t wi = 0; + int32_t bw = 0; + if (common::SerializationUtil::read_i32(wi, in) != common::E_OK || + common::SerializationUtil::read_i32(bw, in) != common::E_OK) { + in.set_read_pos(block_mark); + break; + } + if (wi < 0 || wi > 128 || bw < 0 || bw > bw_limit) { + in.set_read_pos(block_mark); + break; + } + const uint64_t packed_bytes = + (static_cast(wi) * bw + 7) / 8; + if (!skip_bytes(dmfv_bytes) || !skip_bytes(packed_bytes)) { + valid = false; + break; + } + group_value_count += static_cast(wi) + 1; + ++blocks_in_segment; + } + if (!valid || blocks_in_segment == 0) { + valid = false; + break; + } + // The overflow bitmap covers exactly the values of its segment. + if (expected_count != 0 && group_value_count != expected_count) { + valid = false; + break; + } } - return true; + in.set_read_pos(page_start); + return valid && !prefix_offsets.empty(); } +// Consume one Java-compatible segment prefix. Must only be called where +// scan_java_float_double_page() recorded a prefix offset: without the page +// scan there is no reliable way to tell a maxPointNumber varint apart from +// the first byte of a legacy raw block header. inline int consume_float_double_ts2diff_prefix( - common::ByteStream& in, bool& is_legacy_raw, int& max_point_number, + common::ByteStream& in, int& max_point_number, std::vector& underflow_bm, std::vector& overflow_bm, int& segment_size) { int ret = common::E_OK; - is_legacy_raw = false; max_point_number = 0; underflow_bm.clear(); overflow_bm.clear(); segment_size = 0; - uint64_t mark = in.read_pos(); uint32_t tag = 0; if (RET_FAIL(common::SerializationUtil::read_var_uint(tag, in))) { return ret; @@ -285,15 +383,7 @@ inline int consume_float_double_ts2diff_prefix( max_point_number = static_cast(mpn); return common::E_OK; } - - // Distinguish Java maxPointNumber prefix from legacy raw C++ block. max_point_number = static_cast(tag); - if (!looks_like_ts2diff_header(in)) { - in.set_read_pos(mark); - is_legacy_raw = true; - } else { - segment_size = 0; - } return common::E_OK; } @@ -348,6 +438,12 @@ class TS2DIFFDecoder : public Decoder { int64_t value = 0; while (bits > 0) { read_byte_if_empty(in); + // End of input with bits still owed (corrupt or desynced + // stream): bail out instead of looping forever on a stale + // buffer_ / bits_left_ == 0 pair. + if (bits_left_ == 0 && !in.has_remaining()) { + break; + } if (bits > bits_left_ || bits == 8) { // Take only the bits_left_ "least significant" bits. uint8_t d = (uint8_t)(buffer_ & ((1 << bits_left_) - 1)); @@ -933,10 +1029,58 @@ inline int TS2DIFFDecoder::skip_int32(int count, int& skipped, } // ============================================================================ -// Float / Double wrapper decoders (unchanged) +// Float / Double wrapper decoders // ============================================================================ -class FloatTS2DIFFDecoder : public TS2DIFFDecoder { +// Common page-layout detection shared by the FLOAT and DOUBLE decoders. +// A page is either legacy raw (plain delta blocks, written by old C++ +// encoders that bit-cast the float bits into the integer TS_2DIFF stream) +// or Java-compatible (each segment prefixed by a maxPointNumber varint and +// an optional overflow bitmap). The layout is decided once per page by +// parsing the whole page with the Java grammar: a page only counts as +// Java-compatible when the grammar consumes it exactly. This removes the +// old per-block heuristic, which could misclassify a legacy raw header as +// a prefix, desync the stream and spin at end-of-input. +class FloatDoublePageLayout { + protected: + void reset_layout() { + layout_known_ = false; + is_legacy_raw_ = false; + next_prefix_ = 0; + prefix_offsets_.clear(); + } + + // Decide the layout of the page starting at the current read position. + // Safe to call repeatedly before the first value: it always restores + // the read position on exit. + void ensure_layout(common::ByteStream& in, int value_bytes) { + if (!layout_known_) { + is_legacy_raw_ = !ts2diff_java_detail::scan_java_float_double_page( + in, value_bytes, prefix_offsets_); + next_prefix_ = 0; + layout_known_ = true; + } + } + + // True when a Java segment prefix sits at the current read position + // (start of page or start of a new segment). Legacy raw pages never + // match. + bool at_segment_prefix(common::ByteStream& in) { + return !is_legacy_raw_ && next_prefix_ < prefix_offsets_.size() && + prefix_offsets_[next_prefix_] == in.read_pos(); + } + + void advance_segment_prefix() { ++next_prefix_; } + + protected: + bool layout_known_{false}; + bool is_legacy_raw_{false}; + size_t next_prefix_{0}; + std::vector prefix_offsets_; +}; + +class FloatTS2DIFFDecoder : public TS2DIFFDecoder, + protected FloatDoublePageLayout { public: FloatTS2DIFFDecoder() = default; float decode(common::ByteStream& in) { @@ -944,6 +1088,11 @@ class FloatTS2DIFFDecoder : public TS2DIFFDecoder { return common::int_to_float(value_int); } + void reset() override { + TS2DIFFDecoder::reset(); + reset_layout(); + } + int read_boolean(bool& ret_value, common::ByteStream& in) override; int read_int32(int32_t& ret_value, common::ByteStream& in) override; int read_int64(int64_t& ret_value, common::ByteStream& in) override; @@ -952,14 +1101,26 @@ class FloatTS2DIFFDecoder : public TS2DIFFDecoder { int read_batch_float(float* out, int capacity, int& actual, common::ByteStream& in) override { - // FLOAT TS_2DIFF segments have a scale/overflow prefix before the - // integer delta block. The integer batch decoder does not consume - // that prefix, so use the segment-aware scalar decoder here. + // Legacy raw pages are plain int32 delta blocks: reuse the integer + // SIMD batch decoder and bit-cast the results (the layout the old + // C++ writer produced). Java-compatible pages carry segment + // prefixes the integer decoder would misread, so they take the + // segment-aware scalar path. + ensure_layout(in, 4); + if (is_legacy_raw_) { + int32_t* buf = reinterpret_cast(out); + int ret = TS2DIFFDecoder::read_batch_int32(buf, capacity, + actual, in); + if (ret != common::E_OK) return ret; + for (int i = 0; i < actual; ++i) { + out[i] = common::int_to_float(buf[i]); + } + return common::E_OK; + } return Decoder::read_batch_float(out, capacity, actual, in); } private: - bool is_legacy_raw_{false}; int max_point_number_{0}; double max_point_value_{1.0}; int segment_pos_{0}; @@ -968,7 +1129,8 @@ class FloatTS2DIFFDecoder : public TS2DIFFDecoder { std::vector overflow_bm_; }; -class DoubleTS2DIFFDecoder : public TS2DIFFDecoder { +class DoubleTS2DIFFDecoder : public TS2DIFFDecoder, + protected FloatDoublePageLayout { public: DoubleTS2DIFFDecoder() = default; double decode(common::ByteStream& in) { @@ -976,6 +1138,11 @@ class DoubleTS2DIFFDecoder : public TS2DIFFDecoder { return common::long_to_double(value_long); } + void reset() override { + TS2DIFFDecoder::reset(); + reset_layout(); + } + int read_boolean(bool& ret_value, common::ByteStream& in) override; int read_int32(int32_t& ret_value, common::ByteStream& in) override; int read_int64(int64_t& ret_value, common::ByteStream& in) override; @@ -984,14 +1151,22 @@ class DoubleTS2DIFFDecoder : public TS2DIFFDecoder { int read_batch_double(double* out, int capacity, int& actual, common::ByteStream& in) override { - // DOUBLE TS_2DIFF uses the same segment prefix. Bypassing - // read_double() misreads that prefix as a block header and can spin - // at end-of-input while decoding an otherwise valid page. + // Same split as FloatTS2DIFFDecoder::read_batch_float — see there. + ensure_layout(in, 8); + if (is_legacy_raw_) { + int64_t* buf = reinterpret_cast(out); + int ret = TS2DIFFDecoder::read_batch_int64(buf, capacity, + actual, in); + if (ret != common::E_OK) return ret; + for (int i = 0; i < actual; ++i) { + out[i] = common::long_to_double(buf[i]); + } + return common::E_OK; + } return Decoder::read_batch_double(out, capacity, actual, in); } private: - bool is_legacy_raw_{false}; int max_point_number_{0}; double max_point_value_{1.0}; int segment_pos_{0}; @@ -1097,16 +1272,21 @@ FORCE_INLINE int FloatTS2DIFFDecoder::read_float(float& ret_value, common::ByteStream& in) { int ret = common::E_OK; if (current_index_ == 0) { - if (RET_FAIL(ts2diff_java_detail::consume_float_double_ts2diff_prefix( - in, is_legacy_raw_, max_point_number_, underflow_bm_, - overflow_bm_, segment_size_))) { - return ret; + ensure_layout(in, 4); + if (at_segment_prefix(in)) { + if (RET_FAIL( + ts2diff_java_detail::consume_float_double_ts2diff_prefix( + in, max_point_number_, underflow_bm_, overflow_bm_, + segment_size_))) { + return ret; + } + max_point_value_ = + max_point_number_ <= 0 + ? 1.0 + : std::pow(10.0, static_cast(max_point_number_)); + segment_pos_ = 0; + advance_segment_prefix(); } - max_point_value_ = - max_point_number_ <= 0 - ? 1.0 - : std::pow(10.0, static_cast(max_point_number_)); - segment_pos_ = 0; } if (is_legacy_raw_) { ret_value = decode(in); @@ -1158,16 +1338,21 @@ FORCE_INLINE int DoubleTS2DIFFDecoder::read_double(double& ret_value, common::ByteStream& in) { int ret = common::E_OK; if (current_index_ == 0) { - if (RET_FAIL(ts2diff_java_detail::consume_float_double_ts2diff_prefix( - in, is_legacy_raw_, max_point_number_, underflow_bm_, - overflow_bm_, segment_size_))) { - return ret; + ensure_layout(in, 8); + if (at_segment_prefix(in)) { + if (RET_FAIL( + ts2diff_java_detail::consume_float_double_ts2diff_prefix( + in, max_point_number_, underflow_bm_, overflow_bm_, + segment_size_))) { + return ret; + } + max_point_value_ = + max_point_number_ <= 0 + ? 1.0 + : std::pow(10.0, static_cast(max_point_number_)); + segment_pos_ = 0; + advance_segment_prefix(); } - max_point_value_ = - max_point_number_ <= 0 - ? 1.0 - : std::pow(10.0, static_cast(max_point_number_)); - segment_pos_ = 0; } if (is_legacy_raw_) { ret_value = decode(in); diff --git a/cpp/test/encoding/ts2diff_codec_test.cc b/cpp/test/encoding/ts2diff_codec_test.cc index 43c86adef..35aa60d0f 100644 --- a/cpp/test/encoding/ts2diff_codec_test.cc +++ b/cpp/test/encoding/ts2diff_codec_test.cc @@ -523,4 +523,161 @@ TEST(FloatTS2DIFFEncoderResetTest, ResetClearsUnderflowFlags) { } } +// Regression: legacy raw float/double segments (written by the old C++ +// encoders, i.e. plain int delta blocks with no maxPointNumber / overflow +// prefix, values stored as bit-cast float bits) must stay decodable through +// read_batch_float / read_batch_double. The per-block prefix heuristic +// used to misclassify a valid raw header as a maxPointNumber prefix, +// desyncing the stream and spinning at end-of-input (PR #901 review). +TEST_F(FloatDoubleTS2DIFFCodecTest, ReadBatchFloatLegacyRawSegments) { + common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 129; + std::vector expected(row_num); + // 128 equal values followed by a change: the first legacy block has + // bit_width = 0 and delta_min = 0, exactly the pattern the old + // heuristic misclassified. The trailing 1-value block (write_index = 0) + // exercised the same heuristic again. + for (int i = 0; i < row_num; ++i) { + expected[i] = (i < 128) ? 1.5f : 2.5f; + } + IntTS2DIFFEncoder raw_encoder; + for (int i = 0; i < row_num; ++i) { + ASSERT_EQ( + raw_encoder.encode(common::float_to_int(expected[i]), out_stream), + common::E_OK); + } + ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); + + std::vector actual_values(row_num); + int decoded = 0; + // Small batches exercise the layout decision plus repeated block + // transitions. + while (decoded < row_num) { + int actual = 0; + ASSERT_EQ(decoder_float_->read_batch_float( + actual_values.data() + decoded, 16, actual, out_stream), + common::E_OK); + ASSERT_GT(actual, 0); + decoded += actual; + } + for (int i = 0; i < row_num; ++i) { + EXPECT_EQ(actual_values[i], expected[i]) << "row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); +} + +TEST_F(FloatDoubleTS2DIFFCodecTest, ReadBatchDoubleLegacyRawSegments) { + common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 129; + std::vector expected(row_num); + for (int i = 0; i < row_num; ++i) { + expected[i] = (i < 128) ? 1.5 : 2.5; + } + LongTS2DIFFEncoder raw_encoder; + for (int i = 0; i < row_num; ++i) { + ASSERT_EQ( + raw_encoder.encode(common::double_to_long(expected[i]), out_stream), + common::E_OK); + } + ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); + + std::vector actual_values(row_num); + int decoded = 0; + while (decoded < row_num) { + int actual = 0; + ASSERT_EQ(decoder_double_->read_batch_double( + actual_values.data() + decoded, 16, actual, out_stream), + common::E_OK); + ASSERT_GT(actual, 0); + decoded += actual; + } + for (int i = 0; i < row_num; ++i) { + EXPECT_EQ(actual_values[i], expected[i]) << "row " << i; + } + EXPECT_FALSE(decoder_double_->has_remaining(out_stream)); +} + +// The layout decision must also cover the scalar path: legacy raw pages +// read one value at a time via read_float / read_double keep the bit-cast +// semantics across the 128-value block boundary. +TEST_F(FloatDoubleTS2DIFFCodecTest, ReadFloatLegacyRawScalar) { + common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 129; + std::vector expected(row_num); + for (int i = 0; i < row_num; ++i) { + expected[i] = (i < 128) ? 1.5f : 2.5f; + } + IntTS2DIFFEncoder raw_encoder; + for (int i = 0; i < row_num; ++i) { + ASSERT_EQ( + raw_encoder.encode(common::float_to_int(expected[i]), out_stream), + common::E_OK); + } + ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); + + float v = 0.f; + for (int i = 0; i < row_num; ++i) { + ASSERT_EQ(decoder_float_->read_float(v, out_stream), common::E_OK); + EXPECT_EQ(v, expected[i]) << "row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); +} + +TEST_F(FloatDoubleTS2DIFFCodecTest, ReadDoubleLegacyRawScalar) { + common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 129; + std::vector expected(row_num); + for (int i = 0; i < row_num; ++i) { + expected[i] = (i < 128) ? 1.5 : 2.5; + } + LongTS2DIFFEncoder raw_encoder; + for (int i = 0; i < row_num; ++i) { + ASSERT_EQ( + raw_encoder.encode(common::double_to_long(expected[i]), out_stream), + common::E_OK); + } + ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); + + double v = 0.; + for (int i = 0; i < row_num; ++i) { + ASSERT_EQ(decoder_double_->read_double(v, out_stream), common::E_OK); + EXPECT_EQ(v, expected[i]) << "row " << i; + } + EXPECT_FALSE(decoder_double_->has_remaining(out_stream)); +} + +// Mixed reads must not re-trigger the layout scan mid-page: batch first, +// then scalar reads must continue on the same layout decision. +TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyRawBatchThenScalarReads) { + common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 129; + std::vector expected(row_num); + for (int i = 0; i < row_num; ++i) { + expected[i] = (i < 128) ? 0.5f : 100.25f; + } + IntTS2DIFFEncoder raw_encoder; + for (int i = 0; i < row_num; ++i) { + ASSERT_EQ( + raw_encoder.encode(common::float_to_int(expected[i]), out_stream), + common::E_OK); + } + ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); + + float batch_out[40]; + int actual = 0; + ASSERT_EQ( + decoder_float_->read_batch_float(batch_out, 40, actual, out_stream), + common::E_OK); + ASSERT_EQ(actual, 40); + for (int i = 0; i < 40; ++i) { + EXPECT_EQ(batch_out[i], expected[i]) << "row " << i; + } + float v = 0.f; + for (int i = 40; i < row_num; ++i) { + ASSERT_EQ(decoder_float_->read_float(v, out_stream), common::E_OK); + EXPECT_EQ(v, expected[i]) << "row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); +} + } // namespace storage From 67ab361800d0f7e1bd309856fa4c98eab8d5a86c Mon Sep 17 00:00:00 2001 From: gx Date: Wed, 19 Aug 2026 08:12:03 +0800 Subject: [PATCH 03/13] fix(cpp): write maxPointNumber once per page in TS_2DIFF float/double (apache/tsfile#910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of #910: the C++ FloatTS2DIFFEncoder/DoubleTS2DIFFEncoder wrote the maxPointNumber field (fixed value 2) at every segment boundary, while Java FloatEncoder/DoubleEncoder write it only once at the start of each page. Files written with an empty/short first segment could then be misparsed by Java readers (e.g. TsFileSketchTool crashing on the trailing maxPointNumber). This change aligns the C++ encoder with the Java layout: - Encoder: the maxPointNumber var_uint is now emitted exactly once per page (on reset, before segment 1). Segment boundaries only carry the overflow/underflow FLAG when needed, matching Java's segment grammar. - Decoder: forward-only, prefix-aware parsing that accepts all three page layouts — legacy raw pages (no prefix at all), the new Java format (maxPointNumber only on the first segment), and old C++ per-segment format (backward compatible). The old peek-and-rewind scheme is gone; the segment header of a prefix-free segment is preloaded so decode() never needs to re-read the stream. - Tests: new gtest cases assert the maxPointNumber-once-per-page byte layout for multi-segment pages, scaled-overflow pages (the #910 crash scenario), reset() page boundaries, and legacy per-segment backward compatibility. Verified: full C++ test suite passes; Java TsFileSketchTool reads files written by the fixed encoder; tsfile_cli round-trips the data. --- cpp/src/encoding/ts2diff_decoder.h | 512 +++++++++++++----------- cpp/src/encoding/ts2diff_encoder.h | 34 +- cpp/test/encoding/ts2diff_codec_test.cc | 379 +++++++++++++++++- 3 files changed, 676 insertions(+), 249 deletions(-) diff --git a/cpp/src/encoding/ts2diff_decoder.h b/cpp/src/encoding/ts2diff_decoder.h index a3d8f1282..bbf1c310b 100644 --- a/cpp/src/encoding/ts2diff_decoder.h +++ b/cpp/src/encoding/ts2diff_decoder.h @@ -219,137 +219,144 @@ inline bool bitmap_marked(const std::vector& bm, int idx) { return (bm[byte_idx] & static_cast(1u << (idx % 8))) != 0; } -// Parse the remaining stream as one or more Java-compatible FLOAT/DOUBLE -// TS_2DIFF segments, recording the offset of every segment prefix. A -// segment is: -// [overflow flag][value count][underflow bitmap][overflow bitmap?] -// [maxPointNumber varint] block+ -// where a block is [write_index i32][bit_width i32][delta_min][first_value] -// followed by ceil(write_index*bit_width/8) packed bytes. The C++ encoder -// emits one segment per 128-value block, while the Java encoder emits one -// segment wrapping several consecutive blocks, hence "block+". -// -// A legacy raw page (plain delta blocks with no prefix at all) fails this -// parse in practice: its first write_index is >= 1 for any block produced -// by a real encoder, so after the varint tag eats the leading 0x00 byte -// the misaligned write_index probe reads >= 0x100 and is rejected. Only a -// byte-level coincidence could satisfy both interpretations. -// -// Returns true when the whole remaining stream is consumed exactly by the -// segment grammar; the read position is always restored. -inline bool scan_java_float_double_page(common::ByteStream& in, int value_bytes, - std::vector& prefix_offsets) { - const uint64_t page_start = in.read_pos(); - const int bw_limit = (value_bytes == 4) ? 32 : 64; - const int dmfv_bytes = value_bytes * 2; - prefix_offsets.clear(); - - auto skip_bytes = [&in](uint64_t n) -> bool { - uint8_t sink[64]; - while (n > 0) { - uint32_t chunk = n < sizeof(sink) - ? static_cast(n) - : static_cast(sizeof(sink)); - uint32_t got = 0; - if (in.read_buf(sink, chunk, got) != common::E_OK || got != chunk) { - return false; - } - n -= chunk; +inline bool looks_like_ts2diff_header(common::ByteStream& in) { + int ret = common::E_OK; + uint64_t probe_mark = in.read_pos(); + int32_t write_index = 0; + int32_t bit_width = 0; + if (RET_FAIL(common::SerializationUtil::read_i32(write_index, in)) || + RET_FAIL(common::SerializationUtil::read_i32(bit_width, in))) { + in.set_read_pos(probe_mark); + return false; + } + in.set_read_pos(probe_mark); + if (write_index < 0 || write_index > 128) { + return false; + } + if (bit_width < 0 || bit_width > 64) { + return false; + } + return true; +} + +struct SegmentHeaderPreload { + int32_t write_index = 0; + int32_t bit_width = 0; + int64_t delta_min = 0; + int64_t first_value = 0; + bool ready = false; +}; + +// Reads a LEB128 var_uint where the first byte was already consumed into +// `first_byte`. Forward-only: never rewinds the stream. +inline int read_var_uint_tail(uint8_t first_byte, common::ByteStream& in, + uint32_t& out) { + int ret = common::E_OK; + out = static_cast(first_byte & 0x7F); + int shift = 7; + uint8_t b = first_byte; + while (b & 0x80) { + uint32_t read_len = 0; + if (RET_FAIL(in.read_buf(&b, 1, read_len)) || read_len != 1) { + return ret; } - return true; - }; - - bool valid = true; - while (valid && in.has_remaining()) { - prefix_offsets.push_back(in.read_pos()); - uint64_t group_value_count = 0; // sum of (write_index+1) per block - uint64_t expected_count = 0; // bitmap value count, 0 = no bitmap - uint32_t tag = 0; - if (common::SerializationUtil::read_var_uint(tag, in) != common::E_OK) { - valid = false; - break; + if (shift > 28) { + return common::E_TSFILE_CORRUPTED; } - if (tag == FLAG_ORIGINAL_VALUE_OVERFLOW || - tag == FLAG_SCALED_VALUE_OVERFLOW) { - uint32_t n = 0; - if (common::SerializationUtil::read_var_uint(n, in) != - common::E_OK) { - valid = false; - break; - } - expected_count = n; - const uint64_t bm_len = static_cast(n) / 8 + 1; - if (!skip_bytes(bm_len)) { // underflow bitmap - valid = false; - break; - } - if (tag == FLAG_ORIGINAL_VALUE_OVERFLOW && !skip_bytes(bm_len)) { - valid = false; // overflow bitmap - break; - } - uint32_t mpn = 0; - if (common::SerializationUtil::read_var_uint(mpn, in) != - common::E_OK) { - valid = false; - break; - } + out |= static_cast(b & 0x7F) << shift; + shift += 7; + } + return common::E_OK; +} + +// Parses the segment header (write_index + bit_width + delta_min + +// first_value) forward-only. `wi_hi` is the first (already consumed) byte +// of the big-endian write_index - always 0x00 for the no-prefix layout. +inline int read_segment_header_preload(common::ByteStream& in, bool is_double, + uint8_t wi_hi, + SegmentHeaderPreload& h) { + int ret = common::E_OK; + uint8_t rest[3] = {0, 0, 0}; + uint32_t read_len = 0; + if (RET_FAIL(in.read_buf(rest, 3, read_len)) || read_len != 3) { + return ret; + } + h.write_index = (static_cast(wi_hi) << 24) | + (static_cast(rest[0]) << 16) | + (static_cast(rest[1]) << 8) | + static_cast(rest[2]); + int32_t bw = 0; + if (RET_FAIL(common::SerializationUtil::read_i32(bw, in))) { + return ret; + } + h.bit_width = bw; + if (is_double) { + if (RET_FAIL(common::SerializationUtil::read_i64(h.delta_min, in))) { + return ret; } - // Consume the blocks owned by this segment. A block run continues - // while a block header validates; an invalid header marks either - // the next segment prefix or a corrupt page (settled by whether - // the whole-stream parse consumes exactly below). - int blocks_in_segment = 0; - while (true) { - const uint64_t block_mark = in.read_pos(); - int32_t wi = 0; - int32_t bw = 0; - if (common::SerializationUtil::read_i32(wi, in) != common::E_OK || - common::SerializationUtil::read_i32(bw, in) != common::E_OK) { - in.set_read_pos(block_mark); - break; - } - if (wi < 0 || wi > 128 || bw < 0 || bw > bw_limit) { - in.set_read_pos(block_mark); - break; - } - const uint64_t packed_bytes = - (static_cast(wi) * bw + 7) / 8; - if (!skip_bytes(dmfv_bytes) || !skip_bytes(packed_bytes)) { - valid = false; - break; - } - group_value_count += static_cast(wi) + 1; - ++blocks_in_segment; + if (RET_FAIL(common::SerializationUtil::read_i64(h.first_value, in))) { + return ret; } - if (!valid || blocks_in_segment == 0) { - valid = false; - break; + } else { + int32_t dm = 0; + int32_t fv = 0; + if (RET_FAIL(common::SerializationUtil::read_i32(dm, in))) { + return ret; } - // The overflow bitmap covers exactly the values of its segment. - if (expected_count != 0 && group_value_count != expected_count) { - valid = false; - break; + if (RET_FAIL(common::SerializationUtil::read_i32(fv, in))) { + return ret; } + h.delta_min = dm; + h.first_value = fv; } - in.set_read_pos(page_start); - return valid && !prefix_offsets.empty(); + h.ready = true; + return common::E_OK; } -// Consume one Java-compatible segment prefix. Must only be called where -// scan_java_float_double_page() recorded a prefix offset: without the page -// scan there is no reliable way to tell a maxPointNumber varint apart from -// the first byte of a legacy raw block header. inline int consume_float_double_ts2diff_prefix( - common::ByteStream& in, int& max_point_number, - std::vector& underflow_bm, std::vector& overflow_bm, - int& segment_size) { + common::ByteStream& in, bool& is_legacy_raw, bool& max_pn_present, + int& max_point_number, std::vector& underflow_bm, + std::vector& overflow_bm, int& segment_size, + bool page_first_segment, bool is_double, SegmentHeaderPreload& preload) { int ret = common::E_OK; + is_legacy_raw = false; + max_pn_present = true; max_point_number = 0; underflow_bm.clear(); overflow_bm.clear(); segment_size = 0; + uint64_t mark = in.read_pos(); + // apache/tsfile#910 layout: only the page's first segment carries the + // Java maxPointNumber prefix; later segments start directly with the + // 4-byte write_index whose high byte is 0x00. This library always + // serializes max_point_number_ = 2 (0x02), so a leading 0x00 can only + // mean "no prefix on this segment". + // + // Everything is parsed forward-only: rewinding to a page-aligned offset + // (e.g. the start of a page) makes ByteStream::check_space() advance + // the page cursor one page too far and fail the next read, so no + // peek-and-restore is used here. + uint8_t first_byte = 0; + uint32_t read_len = 0; + if (RET_FAIL(in.read_buf(&first_byte, 1, read_len)) || read_len != 1) { + return ret; + } + if (first_byte == 0x00) { + // No prefix: the segment header begins with write_index 0x00... + if (page_first_segment) { + // A page whose very first segment has no prefix is a legacy + // raw C++ block page (no scaling at all). + is_legacy_raw = true; + } + max_pn_present = false; + if (RET_FAIL(read_segment_header_preload(in, is_double, first_byte, + preload))) { + return ret; + } + return common::E_OK; + } uint32_t tag = 0; - if (RET_FAIL(common::SerializationUtil::read_var_uint(tag, in))) { + if (RET_FAIL(read_var_uint_tail(first_byte, in, tag))) { return ret; } if (tag == FLAG_ORIGINAL_VALUE_OVERFLOW || @@ -361,7 +368,6 @@ inline int consume_float_double_ts2diff_prefix( segment_size = static_cast(n); int bm_len = segment_size / 8 + 1; underflow_bm.resize(static_cast(bm_len), 0); - uint32_t read_len = 0; if (RET_FAIL(in.read_buf(underflow_bm.data(), static_cast(bm_len), read_len)) || read_len != static_cast(bm_len)) { @@ -376,17 +382,56 @@ inline int consume_float_double_ts2diff_prefix( return ret; } } + if (page_first_segment) { + // First segment: maxPointNumber always follows the bitmaps. + uint32_t mpn = 0; + if (RET_FAIL(common::SerializationUtil::read_var_uint(mpn, in))) { + return ret; + } + max_point_number = static_cast(mpn); + return common::E_OK; + } + // Later segment: new-format pages jump straight to the segment + // header (0x00 write_index high byte); old-format pages repeat the + // maxPointNumber prefix here. + uint8_t after_bm_byte = 0; + if (RET_FAIL(in.read_buf(&after_bm_byte, 1, read_len)) || + read_len != 1) { + return ret; + } + if (after_bm_byte == 0x00) { + max_pn_present = false; + if (RET_FAIL(read_segment_header_preload(in, is_double, + after_bm_byte, + preload))) { + return ret; + } + return common::E_OK; + } uint32_t mpn = 0; - if (RET_FAIL(common::SerializationUtil::read_var_uint(mpn, in))) { + if (RET_FAIL(read_var_uint_tail(after_bm_byte, in, mpn))) { return ret; } max_point_number = static_cast(mpn); return common::E_OK; } + + // A non-flag tag is the maxPointNumber prefix itself. max_point_number = static_cast(tag); + if (!looks_like_ts2diff_header(in)) { + // Only reachable on corrupt/nonstandard data: a non-flag tag whose + // following bytes are not a valid segment header. Rewind and fall + // back to the raw-block path. The rewind target may be page-aligned + // (e.g. a page start), which trips ByteStream::check_space's page + // cursor - accepted here because valid data never takes this branch. + in.set_read_pos(mark); + is_legacy_raw = true; + max_pn_present = false; + } else { + segment_size = 0; + } return common::E_OK; } - } // namespace ts2diff_java_detail // ============================================================================ @@ -409,6 +454,7 @@ class TS2DIFFDecoder : public Decoder { bit_width_ = 0; current_index_ = 0; header_peeked_ = false; + header_preloaded_ = false; } FORCE_INLINE bool has_remaining(const common::ByteStream& buffer) override { @@ -438,12 +484,6 @@ class TS2DIFFDecoder : public Decoder { int64_t value = 0; while (bits > 0) { read_byte_if_empty(in); - // End of input with bits still owed (corrupt or desynced - // stream): bail out instead of looping forever on a stale - // buffer_ / bits_left_ == 0 pair. - if (bits_left_ == 0 && !in.has_remaining()) { - break; - } if (bits > bits_left_ || bits == 8) { // Take only the bits_left_ "least significant" bits. uint8_t d = (uint8_t)(buffer_ & ((1 << bits_left_) - 1)); @@ -499,6 +539,9 @@ class TS2DIFFDecoder : public Decoder { int write_index_; int current_index_; bool header_peeked_; + // Set when consume_float_double_ts2diff_prefix already parsed the + // segment header (prefix-free segment); decode() must not re-read it. + bool header_preloaded_{false}; }; // ============================================================================ @@ -509,9 +552,15 @@ template <> inline int32_t TS2DIFFDecoder::decode(common::ByteStream& in) { int32_t ret_value = stored_value_; if (UNLIKELY(current_index_ == 0)) { - read_header(in); - common::SerializationUtil::read_i32(delta_min_, in); - common::SerializationUtil::read_i32(first_value_, in); + // A prefix-free segment (no maxPointNumber) has its header parsed + // by consume_float_double_ts2diff_prefix already. + if (UNLIKELY(header_preloaded_)) { + header_preloaded_ = false; + } else { + read_header(in); + common::SerializationUtil::read_i32(delta_min_, in); + common::SerializationUtil::read_i32(first_value_, in); + } ret_value = first_value_; bits_left_ = 0; buffer_ = 0; @@ -538,9 +587,13 @@ template <> inline int64_t TS2DIFFDecoder::decode(common::ByteStream& in) { int64_t ret_value = stored_value_; if (UNLIKELY(current_index_ == 0)) { - read_header(in); - common::SerializationUtil::read_i64(delta_min_, in); - common::SerializationUtil::read_i64(first_value_, in); + if (UNLIKELY(header_preloaded_)) { + header_preloaded_ = false; + } else { + read_header(in); + common::SerializationUtil::read_i64(delta_min_, in); + common::SerializationUtil::read_i64(first_value_, in); + } ret_value = first_value_; if (write_index_ == 0) { current_index_ = 0; @@ -1029,70 +1082,32 @@ inline int TS2DIFFDecoder::skip_int32(int count, int& skipped, } // ============================================================================ -// Float / Double wrapper decoders +// Float / Double wrapper decoders (unchanged) // ============================================================================ -// Common page-layout detection shared by the FLOAT and DOUBLE decoders. -// A page is either legacy raw (plain delta blocks, written by old C++ -// encoders that bit-cast the float bits into the integer TS_2DIFF stream) -// or Java-compatible (each segment prefixed by a maxPointNumber varint and -// an optional overflow bitmap). The layout is decided once per page by -// parsing the whole page with the Java grammar: a page only counts as -// Java-compatible when the grammar consumes it exactly. This removes the -// old per-block heuristic, which could misclassify a legacy raw header as -// a prefix, desync the stream and spin at end-of-input. -class FloatDoublePageLayout { - protected: - void reset_layout() { - layout_known_ = false; - is_legacy_raw_ = false; - next_prefix_ = 0; - prefix_offsets_.clear(); - } - - // Decide the layout of the page starting at the current read position. - // Safe to call repeatedly before the first value: it always restores - // the read position on exit. - void ensure_layout(common::ByteStream& in, int value_bytes) { - if (!layout_known_) { - is_legacy_raw_ = !ts2diff_java_detail::scan_java_float_double_page( - in, value_bytes, prefix_offsets_); - next_prefix_ = 0; - layout_known_ = true; - } - } - - // True when a Java segment prefix sits at the current read position - // (start of page or start of a new segment). Legacy raw pages never - // match. - bool at_segment_prefix(common::ByteStream& in) { - return !is_legacy_raw_ && next_prefix_ < prefix_offsets_.size() && - prefix_offsets_[next_prefix_] == in.read_pos(); - } - - void advance_segment_prefix() { ++next_prefix_; } - - protected: - bool layout_known_{false}; - bool is_legacy_raw_{false}; - size_t next_prefix_{0}; - std::vector prefix_offsets_; -}; - -class FloatTS2DIFFDecoder : public TS2DIFFDecoder, - protected FloatDoublePageLayout { +class FloatTS2DIFFDecoder : public TS2DIFFDecoder { public: FloatTS2DIFFDecoder() = default; + // PageReader invokes reset() at every page boundary; the first segment + // of a page is the only one that may carry the maxPointNumber prefix. + void reset() override { + TS2DIFFDecoder::reset(); + page_first_segment_ = true; + // A legacy raw page sets is_legacy_raw_ for the whole object; clear + // it (and the per-page scale/bitmap state) so a decoder object + // reused across pages stays correct. + is_legacy_raw_ = false; + max_point_value_ = 1.0; + underflow_bm_.clear(); + overflow_bm_.clear(); + segment_pos_ = 0; + segment_size_ = 0; + } float decode(common::ByteStream& in) { int32_t value_int = TS2DIFFDecoder::decode(in); return common::int_to_float(value_int); } - void reset() override { - TS2DIFFDecoder::reset(); - reset_layout(); - } - int read_boolean(bool& ret_value, common::ByteStream& in) override; int read_int32(int32_t& ret_value, common::ByteStream& in) override; int read_int64(int64_t& ret_value, common::ByteStream& in) override; @@ -1101,48 +1116,44 @@ class FloatTS2DIFFDecoder : public TS2DIFFDecoder, int read_batch_float(float* out, int capacity, int& actual, common::ByteStream& in) override { - // Legacy raw pages are plain int32 delta blocks: reuse the integer - // SIMD batch decoder and bit-cast the results (the layout the old - // C++ writer produced). Java-compatible pages carry segment - // prefixes the integer decoder would misread, so they take the - // segment-aware scalar path. - ensure_layout(in, 4); - if (is_legacy_raw_) { - int32_t* buf = reinterpret_cast(out); - int ret = TS2DIFFDecoder::read_batch_int32(buf, capacity, - actual, in); - if (ret != common::E_OK) return ret; - for (int i = 0; i < actual; ++i) { - out[i] = common::int_to_float(buf[i]); - } - return common::E_OK; - } + // FLOAT TS_2DIFF segments have a scale/overflow prefix before the + // integer delta block. The integer batch decoder does not consume + // that prefix, so use the segment-aware scalar decoder here. + // Note: skip_int32/skip_int64 are likewise unsupported on the + // float/double decoders - the segment prefix layout makes the raw + // header-skip path invalid. return Decoder::read_batch_float(out, capacity, actual, in); } private: + bool is_legacy_raw_{false}; int max_point_number_{0}; double max_point_value_{1.0}; int segment_pos_{0}; int segment_size_{0}; std::vector underflow_bm_; std::vector overflow_bm_; + bool page_first_segment_{true}; }; -class DoubleTS2DIFFDecoder : public TS2DIFFDecoder, - protected FloatDoublePageLayout { +class DoubleTS2DIFFDecoder : public TS2DIFFDecoder { public: DoubleTS2DIFFDecoder() = default; + void reset() override { + TS2DIFFDecoder::reset(); + page_first_segment_ = true; + is_legacy_raw_ = false; + max_point_value_ = 1.0; + underflow_bm_.clear(); + overflow_bm_.clear(); + segment_pos_ = 0; + segment_size_ = 0; + } double decode(common::ByteStream& in) { int64_t value_long = TS2DIFFDecoder::decode(in); return common::long_to_double(value_long); } - void reset() override { - TS2DIFFDecoder::reset(); - reset_layout(); - } - int read_boolean(bool& ret_value, common::ByteStream& in) override; int read_int32(int32_t& ret_value, common::ByteStream& in) override; int read_int64(int64_t& ret_value, common::ByteStream& in) override; @@ -1151,28 +1162,23 @@ class DoubleTS2DIFFDecoder : public TS2DIFFDecoder, int read_batch_double(double* out, int capacity, int& actual, common::ByteStream& in) override { - // Same split as FloatTS2DIFFDecoder::read_batch_float — see there. - ensure_layout(in, 8); - if (is_legacy_raw_) { - int64_t* buf = reinterpret_cast(out); - int ret = TS2DIFFDecoder::read_batch_int64(buf, capacity, - actual, in); - if (ret != common::E_OK) return ret; - for (int i = 0; i < actual; ++i) { - out[i] = common::long_to_double(buf[i]); - } - return common::E_OK; - } + // DOUBLE TS_2DIFF uses the same segment prefix. Bypassing + // read_double() misreads that prefix as a block header and can spin + // at end-of-input while decoding an otherwise valid page. + // skip_int32/skip_int64 are likewise unsupported here (see the + // float decoder note). return Decoder::read_batch_double(out, capacity, actual, in); } private: + bool is_legacy_raw_{false}; int max_point_number_{0}; double max_point_value_{1.0}; int segment_pos_{0}; int segment_size_{0}; std::vector underflow_bm_; std::vector overflow_bm_; + bool page_first_segment_{true}; }; typedef TS2DIFFDecoder IntTS2DIFFDecoder; @@ -1271,22 +1277,34 @@ FORCE_INLINE int FloatTS2DIFFDecoder::read_int64(int64_t& ret_value, FORCE_INLINE int FloatTS2DIFFDecoder::read_float(float& ret_value, common::ByteStream& in) { int ret = common::E_OK; - if (current_index_ == 0) { - ensure_layout(in, 4); - if (at_segment_prefix(in)) { - if (RET_FAIL( - ts2diff_java_detail::consume_float_double_ts2diff_prefix( - in, max_point_number_, underflow_bm_, overflow_bm_, - segment_size_))) { - return ret; - } + if (current_index_ == 0 && !is_legacy_raw_) { + bool max_pn_present = true; + ts2diff_java_detail::SegmentHeaderPreload preload; + if (RET_FAIL(ts2diff_java_detail::consume_float_double_ts2diff_prefix( + in, is_legacy_raw_, max_pn_present, max_point_number_, + underflow_bm_, overflow_bm_, segment_size_, page_first_segment_, + false, preload))) { + return ret; + } + // maxPointNumber is written once per page; later segments of + // the page reuse the first segment's scale factor. + if (max_pn_present) { max_point_value_ = max_point_number_ <= 0 ? 1.0 : std::pow(10.0, static_cast(max_point_number_)); - segment_pos_ = 0; - advance_segment_prefix(); } + // Prefix-free segments have their header parsed up front so that + // decode() can pick it up without re-reading the stream. + if (preload.ready) { + write_index_ = preload.write_index; + bit_width_ = preload.bit_width; + delta_min_ = static_cast(preload.delta_min); + first_value_ = static_cast(preload.first_value); + header_preloaded_ = true; + } + page_first_segment_ = false; + segment_pos_ = 0; } if (is_legacy_raw_) { ret_value = decode(in); @@ -1337,22 +1355,32 @@ FORCE_INLINE int DoubleTS2DIFFDecoder::read_float(float& ret_value, FORCE_INLINE int DoubleTS2DIFFDecoder::read_double(double& ret_value, common::ByteStream& in) { int ret = common::E_OK; - if (current_index_ == 0) { - ensure_layout(in, 8); - if (at_segment_prefix(in)) { - if (RET_FAIL( - ts2diff_java_detail::consume_float_double_ts2diff_prefix( - in, max_point_number_, underflow_bm_, overflow_bm_, - segment_size_))) { - return ret; - } + if (current_index_ == 0 && !is_legacy_raw_) { + bool max_pn_present = true; + ts2diff_java_detail::SegmentHeaderPreload preload; + if (RET_FAIL(ts2diff_java_detail::consume_float_double_ts2diff_prefix( + in, is_legacy_raw_, max_pn_present, max_point_number_, + underflow_bm_, overflow_bm_, segment_size_, page_first_segment_, + true, preload))) { + return ret; + } + // maxPointNumber is written once per page; later segments of + // the page reuse the first segment's scale factor. + if (max_pn_present) { max_point_value_ = max_point_number_ <= 0 ? 1.0 : std::pow(10.0, static_cast(max_point_number_)); - segment_pos_ = 0; - advance_segment_prefix(); } + if (preload.ready) { + write_index_ = preload.write_index; + bit_width_ = preload.bit_width; + delta_min_ = preload.delta_min; + first_value_ = preload.first_value; + header_preloaded_ = true; + } + page_first_segment_ = false; + segment_pos_ = 0; } if (is_legacy_raw_) { ret_value = decode(in); diff --git a/cpp/src/encoding/ts2diff_encoder.h b/cpp/src/encoding/ts2diff_encoder.h index fc494581a..a39ed6b5f 100644 --- a/cpp/src/encoding/ts2diff_encoder.h +++ b/cpp/src/encoding/ts2diff_encoder.h @@ -565,6 +565,7 @@ class FloatTS2DIFFEncoder : public TS2DIFFEncoder { void reset() override { TS2DIFFEncoder::reset(); underflow_flags_.clear(); + max_point_number_saved_ = false; } int flush(common::ByteStream& out_stream) override; int encode(bool value, common::ByteStream& out_stream); @@ -609,6 +610,11 @@ class FloatTS2DIFFEncoder : public TS2DIFFEncoder { int max_point_number_; double max_point_value_; std::vector underflow_flags_; + // Java FloatDecoder reads maxPointNumber once per page; this flag + // makes sure only the first 128-value segment of a page carries the + // prefix. PageWriter/ValuePageWriter reset() between pages clears it, + // so every page starts with a fresh prefix (apache/tsfile#910). + bool max_point_number_saved_{false}; }; class DoubleTS2DIFFEncoder : public TS2DIFFEncoder { @@ -623,6 +629,7 @@ class DoubleTS2DIFFEncoder : public TS2DIFFEncoder { void reset() override { TS2DIFFEncoder::reset(); underflow_flags_.clear(); + max_point_number_saved_ = false; } int flush(common::ByteStream& out_stream) override; int encode(bool value, common::ByteStream& out_stream); @@ -667,6 +674,11 @@ class DoubleTS2DIFFEncoder : public TS2DIFFEncoder { int max_point_number_; double max_point_value_; std::vector underflow_flags_; + // Java FloatDecoder reads maxPointNumber once per page; this flag + // makes sure only the first 128-value segment of a page carries the + // prefix. PageWriter/ValuePageWriter reset() between pages clears it, + // so every page starts with a fresh prefix (apache/tsfile#910). + bool max_point_number_saved_{false}; }; typedef TS2DIFFEncoder IntTS2DIFFEncoder; @@ -784,9 +796,14 @@ FORCE_INLINE int FloatTS2DIFFEncoder::flush(common::ByteStream& out_stream) { } const int num_values = write_index_ + 1; common::ByteStream inner(1024, common::MOD_TS2DIFF_OBJ, false); - if (RET_FAIL(common::SerializationUtil::write_var_uint( - static_cast(max_point_number_), inner))) { - return ret; + // Java FloatDecoder reads maxPointNumber only once per page; emit it + // just for the page's first segment (apache/tsfile#910). + if (!max_point_number_saved_) { + if (RET_FAIL(common::SerializationUtil::write_var_uint( + static_cast(max_point_number_), inner))) { + return ret; + } + max_point_number_saved_ = true; } SIMDOps::rebase(delta_arr_, delta_arr_min_, write_index_); int bit_width = cal_bit_width(delta_arr_max_ - delta_arr_min_); @@ -871,9 +888,14 @@ FORCE_INLINE int DoubleTS2DIFFEncoder::flush(common::ByteStream& out_stream) { } const int num_values = write_index_ + 1; common::ByteStream inner(1024, common::MOD_TS2DIFF_OBJ, false); - if (RET_FAIL(common::SerializationUtil::write_var_uint( - static_cast(max_point_number_), inner))) { - return ret; + // Java FloatDecoder reads maxPointNumber only once per page; emit it + // just for the page's first segment (apache/tsfile#910). + if (!max_point_number_saved_) { + if (RET_FAIL(common::SerializationUtil::write_var_uint( + static_cast(max_point_number_), inner))) { + return ret; + } + max_point_number_saved_ = true; } SIMDOps::rebase(delta_arr_, delta_arr_min_, write_index_); int bit_width = cal_bit_width(delta_arr_max_ - delta_arr_min_); diff --git a/cpp/test/encoding/ts2diff_codec_test.cc b/cpp/test/encoding/ts2diff_codec_test.cc index 35aa60d0f..dfb671909 100644 --- a/cpp/test/encoding/ts2diff_codec_test.cc +++ b/cpp/test/encoding/ts2diff_codec_test.cc @@ -680,4 +680,381 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyRawBatchThenScalarReads) { EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); } -} // namespace storage + +// ============================================================================ +// apache/tsfile#910 regression: Java reads the maxPointNumber field only +// once per page (before the first segment); the old C++ encoder repeated it +// at every segment boundary, so multi-segment pages could be misparsed by +// Java readers (TsFileSketchTool / print-tsfile.bat crash). +// +// The fixed encoder emits maxPointNumber exactly once per page (a page +// boundary is signaled by reset()); later segments start directly with the +// 4-byte write_index. The decoder distinguishes the two layouts by peeking +// the byte at the segment start: 0x00 means "no prefix" (write_index high +// byte), any non-zero byte is a var_uint tag (maxPointNumber itself, or the +// overflow flag). +// ============================================================================ + +namespace { + +// LEB128 var_uint, matching common::SerializationUtil::write_var_uint. +bool parse_var_uint(const std::vector& b, size_t& pos, + uint32_t& out) { + if (pos >= b.size()) return false; + out = 0; + int shift = 0; + while (true) { + if (pos >= b.size() || shift > 28) return false; + uint8_t byte = b[pos++]; + out |= static_cast(byte & 0x7F) << shift; + if ((byte & 0x80) == 0) return true; + shift += 7; + } +} + +int32_t read_i32_be(const std::vector& b, size_t pos) { + return (static_cast(b[pos]) << 24) | + (static_cast(b[pos + 1]) << 16) | + (static_cast(b[pos + 2]) << 8) | + static_cast(b[pos + 3]); +} + +// Dumps the full stream content and CONSUMES the stream (read position +// moves to the end). Callers decode from a wrapped copy of the bytes: +// restoring the read position to a page-aligned offset (position 0) makes +// ByteStream::check_space() advance its page cursor one page too far and +// fail the next read, so no rewind is attempted here. +std::vector byte_stream_bytes(common::ByteStream& stream) { + uint32_t size = stream.total_size(); + std::vector buf(size); + uint32_t read_len = 0; + EXPECT_EQ(stream.read_buf(buf.data(), size, read_len), common::E_OK); + EXPECT_EQ(read_len, size); + return buf; +} + +// Wraps dumped page bytes for decoding (same path production chunk readers +// use — a wrapped ByteStream). +void wrap_bytes(const std::vector& b, common::ByteStream& s) { + s.wrap_from(reinterpret_cast(b.data()), + static_cast(b.size())); +} + +// Walks a float/double TS_2DIFF page and asserts the apache/tsfile#910 +// layout invariant: the maxPointNumber var_uint appears only on the page's +// first segment (possibly after a leading overflow-marker section); every +// later segment starts directly with its 4-byte write_index, so any +// non-flag tag on a later segment is a regression. Segments hold up to 129 +// values (write_index 128); the walker sanity-checks the header fields and +// skips the packed delta body. +void expect_max_pn_once_per_page(const std::vector& b, + bool is_double) { + const uint32_t FLAG_SCALED = 2147483647u; + const uint32_t FLAG_ORIGINAL = 2147483646u; + size_t pos = 0; + bool first_segment = true; + int segment_count = 0; + while (pos < b.size()) { + size_t seg_start = pos; + if (pos < b.size() && b[pos] != 0x00) { + uint32_t tag = 0; + size_t p = pos; + ASSERT_TRUE(parse_var_uint(b, p, tag)); + if (tag == FLAG_SCALED || tag == FLAG_ORIGINAL) { + // Overflow marker section: value count + underflow bitmap + // (+ overflow bitmap for original-value overflow). + uint32_t n = 0; + ASSERT_TRUE(parse_var_uint(b, p, n)); + EXPECT_GE(n, 1u); + size_t bm_len = static_cast(n / 8 + 1); + ASSERT_LE(p + bm_len, b.size()); + p += bm_len; + if (tag == FLAG_ORIGINAL) { + ASSERT_LE(p + bm_len, b.size()); + p += bm_len; + } + // Only the page's first segment may carry maxPointNumber + // after the bitmaps. + if (first_segment && p < b.size() && b[p] != 0x00) { + uint32_t mpn = 0; + ASSERT_TRUE(parse_var_uint(b, p, mpn)); + EXPECT_GE(mpn, 1u); + } + pos = p; + } else { + // A non-flag tag is the maxPointNumber prefix; it must not + // appear on any segment after the first. + EXPECT_TRUE(first_segment) + << "maxPointNumber prefix found on segment " + << segment_count + 1 << " (byte " << seg_start << ")"; + pos = p; + } + } + // Segment header: write_index + bit_width (+ delta_min + first_value). + size_t h = pos; + size_t header_len = is_double ? 24 : 16; + ASSERT_LE(h + header_len, b.size()); + int32_t wi = read_i32_be(b, h); + int32_t bw = read_i32_be(b, h + 4); + ASSERT_GE(wi, 0) << "negative write_index at segment " + << segment_count + 1; + EXPECT_LE(wi, 128); + ASSERT_GE(bw, 0); + EXPECT_LE(bw, 64); + pos = h + header_len; + pos += (static_cast(wi) * static_cast(bw) + 7) / 8; + ASSERT_LE(pos, b.size()); + first_segment = false; + segment_count++; + } + EXPECT_GE(segment_count, 2) << "test must produce a multi-segment page"; +} + +} // namespace + +// A page holds multiple 129-value segments; the maxPointNumber must appear +// exactly once, at the page start — not at every segment boundary. +TEST_F(FloatDoubleTS2DIFFCodecTest, MaxPointNumberOncePerPageFloatMultiSegment) { + common::ByteStream out(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 400; // 4 segments: 129 + 129 + 129 + 13 + std::vector data(row_num); + for (int i = 0; i < row_num; i++) { + data[i] = static_cast(i) * 0.25f + 0.5f; + } + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(encoder_float_->encode(data[i], out), common::E_OK); + } + ASSERT_EQ(encoder_float_->flush(out), common::E_OK); + + // Ramp data has bit_width 0, so the only 0x02 byte in the page is the + // maxPointNumber prefix. + std::vector b = byte_stream_bytes(out); + size_t prefix_count = 0; + for (uint8_t byte : b) { + if (byte == 0x02) prefix_count++; + } + EXPECT_EQ(prefix_count, 1u) + << "maxPointNumber must be written once per page, not per segment"; + expect_max_pn_once_per_page(b, false); + + common::ByteStream dec; + wrap_bytes(b, dec); + float x = 0.0f; + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(decoder_float_->read_float(x, dec), common::E_OK); + EXPECT_FLOAT_EQ(x, data[i]) << "row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(dec)); +} + +// Same invariant for the double encoder (i64 delta path). +TEST_F(FloatDoubleTS2DIFFCodecTest, + MaxPointNumberOncePerPageDoubleMultiSegment) { + common::ByteStream out(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 400; + std::vector data(row_num); + for (int i = 0; i < row_num; i++) { + data[i] = static_cast(i) * 0.25 + 0.5; + } + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(encoder_double_->encode(data[i], out), common::E_OK); + } + ASSERT_EQ(encoder_double_->flush(out), common::E_OK); + + std::vector b = byte_stream_bytes(out); + size_t prefix_count = 0; + for (uint8_t byte : b) { + if (byte == 0x02) prefix_count++; + } + EXPECT_EQ(prefix_count, 1u); + expect_max_pn_once_per_page(b, true); + + common::ByteStream dec; + wrap_bytes(b, dec); + double y = 0.0; + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(decoder_double_->read_double(y, dec), common::E_OK); + EXPECT_DOUBLE_EQ(y, data[i]) << "row " << i; + } + EXPECT_FALSE(decoder_double_->has_remaining(dec)); +} + +// The #910 crash scenario: a value that overflows the scaled range in the +// first segment. The overflow-marker section leads the page, the single +// maxPointNumber follows it, and the second segment still has no prefix. +TEST_F(FloatDoubleTS2DIFFCodecTest, + MaxPointNumberOncePerPageFloatWithOverflow) { + common::ByteStream out(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 140; // segment 1 (129 values) + segment 2 (11 values) + std::vector data(row_num); + data[0] = 0.5f; + data[1] = 3.0e7f; // *100 = 3e9 > INT32_MAX → scaled overflow (flag 0) + for (int i = 2; i < row_num; i++) { + data[i] = 0.75f + static_cast(i - 2) * 0.25f; + } + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(encoder_float_->encode(data[i], out), common::E_OK); + } + ASSERT_EQ(encoder_float_->flush(out), common::E_OK); + + // Byte layout: [FLAG var_uint][n=129][underflow bitmap (17B)] + // [maxPointNumber 0x02][seg1 header][packed] + // [seg2 header starting with 0x00 — no prefix] + std::vector b = byte_stream_bytes(out); + size_t pos = 0; + uint32_t tag = 0; + ASSERT_TRUE(parse_var_uint(b, pos, tag)); + EXPECT_EQ(tag, ts2diff_java_detail::FLAG_SCALED_VALUE_OVERFLOW); + uint32_t n = 0; + ASSERT_TRUE(parse_var_uint(b, pos, n)); + EXPECT_EQ(n, 129u); // segment 1 value count + size_t bm_len = static_cast(n / 8 + 1); + ASSERT_LE(pos + bm_len, b.size()); + pos += bm_len; + // Exactly one maxPointNumber, directly after the bitmaps. + ASSERT_LT(pos, b.size()); + EXPECT_EQ(b[pos], 0x02); + uint32_t mpn = 0; + ASSERT_TRUE(parse_var_uint(b, pos, mpn)); + EXPECT_EQ(mpn, 2u); + // Segment 1 header: write_index == 128 (129 values). + ASSERT_LE(pos + 16, b.size()); + int32_t wi = read_i32_be(b, pos); + int32_t bw = read_i32_be(b, pos + 4); + EXPECT_EQ(wi, 128); + pos += 16; + pos += (static_cast(wi) * static_cast(bw) + 7) / 8; + ASSERT_LE(pos, b.size()); + // Segment 2 begins directly with its write_index (0x00 high byte). + EXPECT_EQ(b[pos], 0x00) << "segment 2 must not carry a maxPointNumber"; + + // Round-trip: the overflow value goes through the bitmap path. + common::ByteStream dec; + wrap_bytes(b, dec); + float x = 0.0f; + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(decoder_float_->read_float(x, dec), common::E_OK); + EXPECT_FLOAT_EQ(x, data[i]) << "row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(dec)); +} + +// Same overflow layout for double (scaled > INT64_MAX → 1.0e17 * 100). +// The generic walker understands the FLAG + maxPointNumber + segments +// structure; round-trip goes through the bitmap path. +TEST_F(FloatDoubleTS2DIFFCodecTest, + MaxPointNumberOncePerPageDoubleWithOverflow) { + common::ByteStream out(1024, common::MOD_TS2DIFF_OBJ, false); + const int row_num = 140; + std::vector data(row_num); + data[0] = 0.5; + data[1] = 1.0e17; // *100 = 1e19 > INT64_MAX → scaled overflow (flag 0) + for (int i = 2; i < row_num; i++) { + data[i] = 0.75 + static_cast(i - 2) * 0.25; + } + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(encoder_double_->encode(data[i], out), common::E_OK); + } + ASSERT_EQ(encoder_double_->flush(out), common::E_OK); + + std::vector b = byte_stream_bytes(out); + expect_max_pn_once_per_page(b, true); + + common::ByteStream dec; + wrap_bytes(b, dec); + double y = 0.0; + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(decoder_double_->read_double(y, dec), common::E_OK); + EXPECT_DOUBLE_EQ(y, data[i]) << "row " << i; + } + EXPECT_FALSE(decoder_double_->has_remaining(dec)); +} + +// PageWriter resets the encoder between pages; every page must carry its +// own maxPointNumber prefix (exactly one per page). +TEST_F(FloatDoubleTS2DIFFCodecTest, MaxPointNumberPerPageAfterReset) { + const int row_num = 130; // 129 + 1 → two segments per page + std::vector data(row_num); + for (int i = 0; i < row_num; i++) { + data[i] = static_cast(i) * 0.25f + 0.5f; + } + common::ByteStream page1(1024, common::MOD_TS2DIFF_OBJ, false); + common::ByteStream page2(1024, common::MOD_TS2DIFF_OBJ, false); + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(encoder_float_->encode(data[i], page1), common::E_OK); + } + ASSERT_EQ(encoder_float_->flush(page1), common::E_OK); + encoder_float_->reset(); // page boundary + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(encoder_float_->encode(data[i], page2), common::E_OK); + } + ASSERT_EQ(encoder_float_->flush(page2), common::E_OK); + + std::vector b1 = byte_stream_bytes(page1); + std::vector b2 = byte_stream_bytes(page2); + size_t c1 = 0; + size_t c2 = 0; + for (uint8_t byte : b1) { + if (byte == 0x02) c1++; + } + for (uint8_t byte : b2) { + if (byte == 0x02) c2++; + } + EXPECT_EQ(c1, 1u) << "page 1 must carry exactly one maxPointNumber"; + EXPECT_EQ(c2, 1u) << "page 2 must carry exactly one maxPointNumber"; + expect_max_pn_once_per_page(b1, false); + expect_max_pn_once_per_page(b2, false); + + // Both pages decode with the same decoder; PageReader calls reset() + // between pages, which must re-arm the per-page prefix state. + common::ByteStream d1; + common::ByteStream d2; + wrap_bytes(b1, d1); + wrap_bytes(b2, d2); + float x = 0.0f; + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(decoder_float_->read_float(x, d1), common::E_OK); + EXPECT_FLOAT_EQ(x, data[i]) << "page1 row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(d1)); + decoder_float_->reset(); // page boundary + for (int i = 0; i < row_num; i++) { + ASSERT_EQ(decoder_float_->read_float(x, d2), common::E_OK); + EXPECT_FLOAT_EQ(x, data[i]) << "page2 row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(d2)); +} + +// Backward compatibility: files written by the pre-#910 encoder carry the +// maxPointNumber at every segment boundary. The decoder must keep reading +// them (it rescales whenever a prefix is present instead of assuming +// once-per-page). +TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyPerSegmentMaxPNStillDecodes) { + // Build an old-format page by hand: 0x02 prefix before BOTH segments. + const std::vector expected = {0.5f, 0.75f, 1.0f, 1.25f, + 1.5f, 1.75f, 2.0f, 2.25f}; + common::ByteStream old_fmt(1024, common::MOD_TS2DIFF_OBJ, false); + // Segment 1: 6 values (first 50, five deltas of 25), bit_width 0. + ASSERT_EQ(common::SerializationUtil::write_var_uint(2, old_fmt), + common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(5, old_fmt), common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(0, old_fmt), common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(25, old_fmt), common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(50, old_fmt), common::E_OK); + // Segment 2: 2 values (first 200, one delta of 25) — WITH prefix again. + ASSERT_EQ(common::SerializationUtil::write_var_uint(2, old_fmt), + common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(1, old_fmt), common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(0, old_fmt), common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(25, old_fmt), common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(200, old_fmt), common::E_OK); + + float x = 0.0f; + for (size_t i = 0; i < expected.size(); i++) { + ASSERT_EQ(decoder_float_->read_float(x, old_fmt), common::E_OK); + EXPECT_FLOAT_EQ(x, expected[i]) << "row " << i; + } + EXPECT_FALSE(decoder_float_->has_remaining(old_fmt)); +} + +} // namespace storage \ No newline at end of file From 838a3fa03154de746e6252b3331270e0674e4e40 Mon Sep 17 00:00:00 2001 From: gx Date: Wed, 19 Aug 2026 21:50:31 +0800 Subject: [PATCH 04/13] style(cpp): fix spotless clang-format violations in ts2diff files --- cpp/src/encoding/ts2diff_decoder.h | 6 ++---- cpp/test/encoding/ts2diff_codec_test.cc | 14 +++++++------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/cpp/src/encoding/ts2diff_decoder.h b/cpp/src/encoding/ts2diff_decoder.h index bbf1c310b..1175cf4b9 100644 --- a/cpp/src/encoding/ts2diff_decoder.h +++ b/cpp/src/encoding/ts2diff_decoder.h @@ -273,8 +273,7 @@ inline int read_var_uint_tail(uint8_t first_byte, common::ByteStream& in, // first_value) forward-only. `wi_hi` is the first (already consumed) byte // of the big-endian write_index - always 0x00 for the no-prefix layout. inline int read_segment_header_preload(common::ByteStream& in, bool is_double, - uint8_t wi_hi, - SegmentHeaderPreload& h) { + uint8_t wi_hi, SegmentHeaderPreload& h) { int ret = common::E_OK; uint8_t rest[3] = {0, 0, 0}; uint32_t read_len = 0; @@ -402,8 +401,7 @@ inline int consume_float_double_ts2diff_prefix( if (after_bm_byte == 0x00) { max_pn_present = false; if (RET_FAIL(read_segment_header_preload(in, is_double, - after_bm_byte, - preload))) { + after_bm_byte, preload))) { return ret; } return common::E_OK; diff --git a/cpp/test/encoding/ts2diff_codec_test.cc b/cpp/test/encoding/ts2diff_codec_test.cc index dfb671909..07fdd94a3 100644 --- a/cpp/test/encoding/ts2diff_codec_test.cc +++ b/cpp/test/encoding/ts2diff_codec_test.cc @@ -680,7 +680,6 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyRawBatchThenScalarReads) { EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); } - // ============================================================================ // apache/tsfile#910 regression: Java reads the maxPointNumber field only // once per page (before the first segment); the old C++ encoder repeated it @@ -698,8 +697,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyRawBatchThenScalarReads) { namespace { // LEB128 var_uint, matching common::SerializationUtil::write_var_uint. -bool parse_var_uint(const std::vector& b, size_t& pos, - uint32_t& out) { +bool parse_var_uint(const std::vector& b, size_t& pos, uint32_t& out) { if (pos >= b.size()) return false; out = 0; int shift = 0; @@ -814,7 +812,8 @@ void expect_max_pn_once_per_page(const std::vector& b, // A page holds multiple 129-value segments; the maxPointNumber must appear // exactly once, at the page start — not at every segment boundary. -TEST_F(FloatDoubleTS2DIFFCodecTest, MaxPointNumberOncePerPageFloatMultiSegment) { +TEST_F(FloatDoubleTS2DIFFCodecTest, + MaxPointNumberOncePerPageFloatMultiSegment) { common::ByteStream out(1024, common::MOD_TS2DIFF_OBJ, false); const int row_num = 400; // 4 segments: 129 + 129 + 129 + 13 std::vector data(row_num); @@ -1031,8 +1030,8 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, MaxPointNumberPerPageAfterReset) { // once-per-page). TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyPerSegmentMaxPNStillDecodes) { // Build an old-format page by hand: 0x02 prefix before BOTH segments. - const std::vector expected = {0.5f, 0.75f, 1.0f, 1.25f, - 1.5f, 1.75f, 2.0f, 2.25f}; + const std::vector expected = {0.5f, 0.75f, 1.0f, 1.25f, + 1.5f, 1.75f, 2.0f, 2.25f}; common::ByteStream old_fmt(1024, common::MOD_TS2DIFF_OBJ, false); // Segment 1: 6 values (first 50, five deltas of 25), bit_width 0. ASSERT_EQ(common::SerializationUtil::write_var_uint(2, old_fmt), @@ -1047,7 +1046,8 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyPerSegmentMaxPNStillDecodes) { ASSERT_EQ(common::SerializationUtil::write_ui32(1, old_fmt), common::E_OK); ASSERT_EQ(common::SerializationUtil::write_ui32(0, old_fmt), common::E_OK); ASSERT_EQ(common::SerializationUtil::write_ui32(25, old_fmt), common::E_OK); - ASSERT_EQ(common::SerializationUtil::write_ui32(200, old_fmt), common::E_OK); + ASSERT_EQ(common::SerializationUtil::write_ui32(200, old_fmt), + common::E_OK); float x = 0.0f; for (size_t i = 0; i < expected.size(); i++) { From bf5be1672ce7ab4407a38d1d3d528afa7f48cbdc Mon Sep 17 00:00:00 2001 From: gx Date: Sat, 22 Aug 2026 18:59:11 +0800 Subject: [PATCH 05/13] fix(windows): open file paths as UTF-8 via _wopen (port of vendored patch) CRT ::open interprets bytes in the active code page; UTF-8 paths with non-ASCII characters fail with E_FILE_OPEN_ERR (28) on machines where the 8.3-shortpath / ACP-transcode workarounds unavailable (8dot3 disabled on the volume, or ACP cannot represent the characters). file_internal::open_utf8 converts UTF-8 -> wide chars -> _wopen, same as the vendored TsFileCpp tree. Applied to ReadFile::open, WriteFile, and RestorableTsFileIOWriter's self-check reader. --- cpp/src/file/read_file.cc | 3 +- cpp/src/file/restorable_tsfile_io_writer.cc | 3 +- cpp/src/file/utf8_file_open.h | 69 +++++++++++++++++++++ cpp/src/file/write_file.cc | 3 +- 4 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 cpp/src/file/utf8_file_open.h diff --git a/cpp/src/file/read_file.cc b/cpp/src/file/read_file.cc index ce1f67197..2cc91bc45 100644 --- a/cpp/src/file/read_file.cc +++ b/cpp/src/file/read_file.cc @@ -34,6 +34,7 @@ ssize_t pread(int fd, void* buf, size_t count, uint64_t offset); #include "common/logger/elog.h" #include "common/tsfile_common.h" +#include "file/utf8_file_open.h" #include "utils/util_define.h" // ssize_t and other platform-compat shims using namespace common; @@ -103,7 +104,7 @@ int ReadFile::open(const std::string& file_path) { #ifdef _WIN32 flags |= O_BINARY; #endif - fd_ = ::open(file_path_.c_str(), flags); + fd_ = file_internal::open_utf8(file_path_, flags); if (fd_ < 0) { std::cerr << "open file " << file_path << " error: " << strerror(errno) << " (errno " << errno << ")" << std::endl; diff --git a/cpp/src/file/restorable_tsfile_io_writer.cc b/cpp/src/file/restorable_tsfile_io_writer.cc index a1fc53402..a0b0d7313 100644 --- a/cpp/src/file/restorable_tsfile_io_writer.cc +++ b/cpp/src/file/restorable_tsfile_io_writer.cc @@ -18,6 +18,7 @@ */ #include "file/restorable_tsfile_io_writer.h" +#include "file/utf8_file_open.h" #include @@ -96,7 +97,7 @@ struct SelfCheckReader { #ifdef _WIN32 fd_ = ::_open(path.c_str(), _O_RDONLY | _O_BINARY); #else - fd_ = ::open(path.c_str(), O_RDONLY); + fd_ = file_internal::open_utf8(path, O_RDONLY); #endif if (fd_ < 0) { return E_FILE_OPEN_ERR; diff --git a/cpp/src/file/utf8_file_open.h b/cpp/src/file/utf8_file_open.h new file mode 100644 index 000000000..7148fe592 --- /dev/null +++ b/cpp/src/file/utf8_file_open.h @@ -0,0 +1,69 @@ +/* + * 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. + */ + +#pragma once + +#include +#include +#include +#include + +#ifdef _WIN32 +#include +#include +#else +#include +#endif + +namespace storage { +namespace file_internal { + +inline int open_utf8(const std::string& path, int flags, int mode = 0) { +#ifdef _WIN32 + if (path.find('\0') != std::string::npos || path.size() > INT_MAX) { + errno = EINVAL; + return -1; + } + if (path.empty()) { + errno = ENOENT; + return -1; + } + const int size = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, + path.data(), + static_cast(path.size()), nullptr, + 0); + if (size <= 0) { + errno = EINVAL; + return -1; + } + std::wstring wide_path(static_cast(size), L'\0'); + if (MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, path.data(), + static_cast(path.size()), &wide_path[0], + size) != size) { + errno = EINVAL; + return -1; + } + return ::_wopen(wide_path.c_str(), flags, mode); +#else + return ::open(path.c_str(), flags, mode); +#endif +} + +} // namespace file_internal +} // namespace storage diff --git a/cpp/src/file/write_file.cc b/cpp/src/file/write_file.cc index 68ac127ac..c2eb58ce2 100644 --- a/cpp/src/file/write_file.cc +++ b/cpp/src/file/write_file.cc @@ -18,6 +18,7 @@ */ #include "write_file.h" +#include "file/utf8_file_open.h" #include #include @@ -59,7 +60,7 @@ int WriteFile::do_create(int flags, mode_t mode) { flags |= O_BINARY; #endif // TODO make sure no same file exists - fd_ = ::open(path_.c_str(), flags, mode); + fd_ = file_internal::open_utf8(path_, flags, mode); if (fd_ < 0) { // log_err("open file error, path=%s, errno=%d", path_.c_str(), errno); ret = E_FILE_OPEN_ERR; From 746d314b1e2be36b7b0198f41a36aece73b34fc2 Mon Sep 17 00:00:00 2001 From: gx Date: Sat, 22 Aug 2026 19:26:48 +0800 Subject: [PATCH 06/13] fix: place utf8_file_open.h after the tsfile headers in restorable writer windows.h from utf8_file_open.h before decoder_factory.h made INT32/ DATE/DOUBLE ambiguous with using-namespace common in the decoder switch. --- cpp/src/file/restorable_tsfile_io_writer.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/file/restorable_tsfile_io_writer.cc b/cpp/src/file/restorable_tsfile_io_writer.cc index a0b0d7313..347caece8 100644 --- a/cpp/src/file/restorable_tsfile_io_writer.cc +++ b/cpp/src/file/restorable_tsfile_io_writer.cc @@ -18,7 +18,6 @@ */ #include "file/restorable_tsfile_io_writer.h" -#include "file/utf8_file_open.h" #include @@ -45,6 +44,7 @@ ssize_t pread(int fd, void* buf, size_t count, uint64_t offset); #include #endif +#include "file/utf8_file_open.h" using namespace common; namespace storage { From c652fcc57c779dcbb2dd4aacda23b481ca083925 Mon Sep 17 00:00:00 2001 From: gx Date: Sat, 22 Aug 2026 20:40:02 +0800 Subject: [PATCH 07/13] fix(reader): report the file's actual encoding/compression in schema get_timeseries_schema built MeasurementSchema with the 2-arg ctor, whose encoding/compression are library defaults (DOUBLE->GORILLA, LZ4) rather than what the file stores. Take both from the first ChunkMeta of the timeseries (chunk metadata is deserialized from the file), falling back to defaults when no chunk metadata is available. --- cpp/src/reader/tsfile_reader.cc | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/cpp/src/reader/tsfile_reader.cc b/cpp/src/reader/tsfile_reader.cc index 6e20b2d63..4c9f0a605 100644 --- a/cpp/src/reader/tsfile_reader.cc +++ b/cpp/src/reader/tsfile_reader.cc @@ -471,9 +471,34 @@ int TsFileReader::get_timeseries_schema( dt = aligned->value_ts_idx_->get_data_type(); } } - MeasurementSchema ms( - timeseries_index->get_measurement_name().to_std_string(), dt); - result.push_back(ms); + // Report the encoding/compression the file actually stores + // (from the chunk metadata), not library defaults. The 2-arg + // MeasurementSchema ctor fills get_value_encoder(dt) / + // get_default_compressor(), which mislabels e.g. a TS_2DIFF + // UNCOMPRESSED column as GORILLA/LZ4. + common::TSEncoding enc = common::INVALID_ENCODING; + common::CompressionType comp = common::INVALID_COMPRESSION; + auto* chunk_meta_list = timeseries_index->get_chunk_meta_list(); + if (chunk_meta_list != nullptr && chunk_meta_list->size() > 0) { + ChunkMeta* first_meta = chunk_meta_list->front(); + if (first_meta != nullptr) { + enc = first_meta->encoding_; + comp = first_meta->compression_type_; + } + } + if (enc == common::INVALID_ENCODING || + comp == common::INVALID_COMPRESSION) { + // No chunk metadata available: fall back to defaults. + MeasurementSchema ms( + timeseries_index->get_measurement_name().to_std_string(), + dt); + result.push_back(ms); + } else { + MeasurementSchema ms( + timeseries_index->get_measurement_name().to_std_string(), + dt, enc, comp); + result.push_back(ms); + } } } return E_OK; From 58c6a226b402cec557df0b2a3dd968733ee56317 Mon Sep 17 00:00:00 2001 From: gx Date: Sat, 22 Aug 2026 20:53:59 +0800 Subject: [PATCH 08/13] fix(reader): schema reads real encoding/compression from chunk header bytes ChunkMeta entries from the metadata index carry only offsets (C++ deserialization never fills encoding_/compression_type_, unlike Java), so the previous attempt read uninitialized memory. Now: read 256 bytes at the first chunk's offset_of_chunk_header_ and deserialize the ChunkHeader (encoding/compression live there). Adds TsFileIOReader::get_read_file(). --- cpp/src/file/tsfile_io_reader.h | 4 +++ cpp/src/reader/tsfile_reader.cc | 43 +++++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/cpp/src/file/tsfile_io_reader.h b/cpp/src/file/tsfile_io_reader.h index 9da3b52e9..c50e1b061 100644 --- a/cpp/src/file/tsfile_io_reader.h +++ b/cpp/src/file/tsfile_io_reader.h @@ -112,6 +112,10 @@ class TsFileIOReader { std::string get_file_path() const { return read_file_->file_path(); } + // Raw read access for callers that need file bytes (e.g. parsing chunk + // headers at offsets from chunk metadata). + ReadFile* get_read_file() const { return read_file_; } + TsFileMeta* get_tsfile_meta() { load_tsfile_meta_if_necessary(); return &tsfile_meta_; diff --git a/cpp/src/reader/tsfile_reader.cc b/cpp/src/reader/tsfile_reader.cc index 4c9f0a605..895bbccf3 100644 --- a/cpp/src/reader/tsfile_reader.cc +++ b/cpp/src/reader/tsfile_reader.cc @@ -20,6 +20,9 @@ #include +#include "file/read_file.h" +#include "common/tsfile_common.h" +#include "common/allocator/byte_stream.h" #include "common/schema.h" #include "filter/time_operator.h" #include "tsfile_executor.h" @@ -471,19 +474,39 @@ int TsFileReader::get_timeseries_schema( dt = aligned->value_ts_idx_->get_data_type(); } } - // Report the encoding/compression the file actually stores - // (from the chunk metadata), not library defaults. The 2-arg - // MeasurementSchema ctor fills get_value_encoder(dt) / - // get_default_compressor(), which mislabels e.g. a TS_2DIFF - // UNCOMPRESSED column as GORILLA/LZ4. + // Report the encoding/compression the file actually stores, + // not library defaults. The 2-arg MeasurementSchema ctor fills + // get_value_encoder(dt) / get_default_compressor(), which + // mislabels e.g. a TS_2DIFF UNCOMPRESSED column as GORILLA/LZ4. + // ChunkMeta entries deserialized by the metadata index carry + // only offsets (their encoding_ fields are uninitialized), so + // read the first chunk's header bytes from the file at that + // offset — the ChunkHeader serialization carries the real + // encoding/compression. common::TSEncoding enc = common::INVALID_ENCODING; common::CompressionType comp = common::INVALID_COMPRESSION; auto* chunk_meta_list = timeseries_index->get_chunk_meta_list(); - if (chunk_meta_list != nullptr && chunk_meta_list->size() > 0) { - ChunkMeta* first_meta = chunk_meta_list->front(); - if (first_meta != nullptr) { - enc = first_meta->encoding_; - comp = first_meta->compression_type_; + if (chunk_meta_list != nullptr && chunk_meta_list->size() > 0 && + chunk_meta_list->front() != nullptr) { + const int64_t chunk_header_offset = + chunk_meta_list->front()->offset_of_chunk_header_; + ReadFile* rf = tsfile_executor_->get_tsfile_io_reader() + ->get_read_file(); + if (rf != nullptr && chunk_header_offset >= 0) { + char buf[256]; + int32_t read_len = 0; + if (rf->read(chunk_header_offset, buf, sizeof(buf), + read_len) == E_OK && + read_len > 0) { + common::ByteStream in( + read_len, common::MOD_TSFILE_READER, false); + in.wrap_from(buf, read_len); + ChunkHeader ch; + if (ch.deserialize_from(in) == E_OK) { + enc = ch.encoding_type_; + comp = ch.compression_type_; + } + } } } if (enc == common::INVALID_ENCODING || From 3a5db160e48b0bc582a8dc676c5215010f3c7d53 Mon Sep 17 00:00:00 2001 From: gx Date: Wed, 26 Aug 2026 11:23:51 +0800 Subject: [PATCH 09/13] test(compat): extend matrix with FLOAT/DOUBLE TS_2DIFF and 300-row fixtures Extend the Java and C++ encoding/compression compatibility matrices with FLOAT + TS_2DIFF and DOUBLE + TS_2DIFF cases and raise every case to 300 rows so pages cross the 129-value TS_2DIFF block boundary (129+129+42), per review feedback on apache/tsfile#901. The TS_2DIFF value set covers all page layouts the writers can produce: scaled integers, scale-overflow values (reachable at maxPointNumber 2, the C++ writer), and raw IEEE bit patterns (NaN/Infinity, two page-wide bitmaps). Every chosen value restores identically whether the writer used maxPointNumber 0 (Java builder default) or 2 (historical C++ default), so the validating reader never needs to know which writer produced a file; NaN expectations use the canonical Java floatToIntBits pattern. Expected values are computed by applying the writer's tri-state conversion rules, not by reusing the input bits, so non-integer inputs would not round-trip exactly and are excluded. Also add a wire-format contract document derived from the Java FloatEncoder/FloatDecoder/DeltaBinaryEncoder reference implementations (cpp/docs/ts2diff-float-double-wire-format.md), which the upcoming encoder/decoder rework will be validated against. Current state: the six new TS_2DIFF float/double cases fail on the C++ side (write_table returns E_INVALID_ARG and decoded values are misaligned), which is the acceptance baseline the rework must turn green. --- cpp/docs/ts2diff-float-double-wire-format.md | 132 +++++++++++++++ ...encoding_compression_compatibility_test.cc | 158 +++++++++++++++++- ...lEncodingCompressionCompatibilityTest.java | 112 ++++++++++++- 3 files changed, 386 insertions(+), 16 deletions(-) create mode 100644 cpp/docs/ts2diff-float-double-wire-format.md diff --git a/cpp/docs/ts2diff-float-double-wire-format.md b/cpp/docs/ts2diff-float-double-wire-format.md new file mode 100644 index 000000000..4cdb48d7e --- /dev/null +++ b/cpp/docs/ts2diff-float-double-wire-format.md @@ -0,0 +1,132 @@ + + +# FLOAT/DOUBLE TS_2DIFF Wire Format (Java Canonical Layout) + +This document specifies the canonical on-disk layout of FLOAT/DOUBLE TS_2DIFF +pages, derived from the Java reference implementation +(`FloatEncoder`, `FloatDecoder`, `DeltaBinaryEncoder`, `BitMap`). +The Java layout is the cross-language compatibility boundary. Other layouts +produced by earlier C++ writers (raw bit-cast, per-block wrapper metadata) are +implementation artifacts outside the compatibility scope; the C++ decoder +treats them as a format error. + +## Encoding Pipeline + +TS_2DIFF encodes integers. Floating-point values go through a wrapper that +converts each value to an integer, encodes the integers with +`IntDeltaEncoder` (FLOAT) or `LongDeltaEncoder` (DOUBLE), and emits page-wide +conversion metadata. + +Given `maxPointNumber = mpn` and `maxPointValue = 10^mpn` (`mpn <= 0` implies +`maxPointValue = 1`), each value maps to one of three stored forms: + +| Condition | Stored bits | Decoder action | +| -------------------------------------- | --------------------------- | ------------------------- | +| `round(v * 10^mpn)` fits the int type | `round(v * 10^mpn)` | divide by `10^mpn` | +| scaled overflows but `v` itself fits | `round(v)` | divide by `1` | +| `v` out of int range, or NaN | `floatToIntBits(v)` / `doubleToLongBits(v)` | restore raw bits | + +The three forms are tracked per page as a tri-state flag list +(`underflowFlags` in Java): + +- `true` -> scaled form +- `false` -> rounded form (scale overflow) +- `null` -> raw IEEE 754 bits (value overflow or NaN) + +## Page Layout + +```text +# Form 1: every value stored in scaled form (no bitmap at all) +[maxPointNumber varint] +[TS_2DIFF block 1][TS_2DIFF block 2]...[final block] + +# Form 2: at least one value is 'false' (scale overflow), none is 'null' +[Integer.MAX_VALUE varint] # 0xFF 0xFF 0xFF 0xFF 0x07 +[pageValueCount varint] +[scaled-bitmap, pageValueCount/8+1 bytes] # marks 'true' entries +[maxPointNumber varint] +[TS_2DIFF block 1]...[final block] + +# Form 3: at least one value is 'null' (raw bits) +[Integer.MAX_VALUE-1 varint] # 0xFF 0xFF 0xFF 0xFF 0x06 +[pageValueCount varint] +[scaled-bitmap, pageValueCount/8+1 bytes] # marks 'true' entries +[raw-bitmap, pageValueCount/8+1 bytes] # marks 'null' entries +[maxPointNumber varint] +[TS_2DIFF block 1]...[final block] +``` + +Key invariants: + +- `maxPointNumber` appears exactly once per page, before the first integer + block (for Forms 2/3 it appears after the bitmaps). +- The bitmaps cover the entire page, not individual TS_2DIFF blocks. The + decoder keeps one page-wide `position` that never resets between blocks; + only a page-level `reset()` clears it. +- Bitmap byte length is always `size/8 + 1`, even when `size % 8 == 0` + (`BitMap.getSizeOfBytes`). +- Bitmap bit order is LSB-first within each byte: position `p` maps to + `bits[p / 8] & (1 << (p % 8))`. +- `pageValueCount` counts all values of the page (across blocks). +- A first-page byte of `0x00` is the normal encoding of `maxPointNumber = 0`, + which is the Java `Ts2Diff` builder default. It is not a legacy marker. + +## Integer Block Layout + +Identical to the integer TS_2DIFF format (`DeltaBinaryEncoder`): + +```text +[writeIndex int32 BE] # number of values in this block (<= 129) +[bitWidth int32 BE] +[block-specific header] # first value; min delta +[packed data] # writeIndex * bitWidth bits +``` + +`BLOCK_DEFAULT_SIZE = 128`: the encoder buffers the first value plus up to 128 +deltas, then flushes a 129-value block. A 300-value page therefore produces +blocks of 129, 129, 42. + +## Encoder Construction + +Java `TSEncodingBuilder.Ts2Diff` hard-codes `maxPointNumber = 0` for +FLOAT/DOUBLE (it does not read `max_point_number` props). Pages produced by +Java therefore start with `0x00`. The value stored in the stream is +self-describing, so writers using other `maxPointNumber` values remain +readable, but byte-level fixture parity with Java requires `mpn = 0`. + +## Decoder State Machine + +Per page, exactly once, the decoder reads the leading marker: + +1. Read varint `tag`. +2. `tag == Integer.MAX_VALUE` -> read `count` varint, `count/8+1` bytes + scaled-bitmap, then varint `maxPointNumber` (Form 2). +3. `tag == Integer.MAX_VALUE-1` -> additionally read a second + `count/8+1` bytes raw-bitmap (Form 3). +4. Otherwise `tag` itself is `maxPointNumber` (Form 1); `mpn <= 0` means + `maxPointValue = 1`. + +Then values are decoded from the integer blocks. For value at page position +`p`: + +- raw-bitmap (if present) marks `p` -> `intBitsToFloat` / `longBitsToDouble` +- else scaled-bitmap (if present) marks `p` -> `value / 10^mpn` +- else -> `value / 1` + +Any input that does not conform to this grammar (for example, an integer +TS_2DIFF block header where the page metadata is expected) is a format error. diff --git a/cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc b/cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc index 5db8266b0..1b2ef374b 100644 --- a/cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc +++ b/cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -65,7 +66,10 @@ const char* const kTableName = "compat_table"; const char* const kTagColumn = "device"; const char* const kValueColumn = "value"; const char* const kTagValue = "compat_device"; -constexpr int kRowCount = 32; +// Crosses the 129-value TS_2DIFF block boundary (300 -> 129+129+42), so +// page-wide bitmaps must survive block transitions. Applied to every +// case, per review feedback on apache/tsfile#901. +constexpr int kRowCount = 300; const int32_t kIntValues[] = { 0, @@ -123,6 +127,44 @@ const uint64_t kDoubleBits[] = { UINT64_C(0x405edd2f1a9fbe77), UINT64_C(0xc05edd2f1a9fbe77), }; +// FLOAT/DOUBLE + TS_2DIFF converts values to fixed-point with +// maxPointNumber (10^mpn). The expected value is computed by applying the +// same tri-state conversion the writer performs and then decoding it back, +// so expectations reflect the encoding rules rather than the raw input +// bits. Every chosen value has the same expected value whether the writer +// used maxPointNumber 0 (the Java Ts2Diff builder default) or 2 (the +// historical C++ default), so the validating reader never needs to know +// which writer produced the file. The values cover all page layouts the +// writers can produce: +// - plain integers (scaled form; at mpn = 0 every finite in-range value +// is scaled, so pages written by the Java builder contain no bitmap) +// - 1.5e9f / 1e18: the scaled product overflows while round(v) still +// fits -> scale-overflow form, single page-wide bitmap (only reachable +// at mpn > 0, i.e. the C++ writer) +// - NaN and +/-Infinity (stored as raw IEEE bits -> two page-wide +// bitmaps; NaN uses the canonical Java floatToIntBits pattern) +const uint32_t kTs2DiffFloatBits[] = { + 0x00000000U, 0x3f800000U, 0xbf800000U, 0x461c4000U, + 0xc61c4000U, 0x4eb2d05eU, 0x7f800000U, 0xff800000U, + 0x7fc00000U, 0x3f800000U, 0x00000000U, 0x4a742400U, + 0xca742400U, 0x4e6e6b28U, 0x3f800000U, 0x00000000U, +}; + +const uint64_t kTs2DiffDoubleBits[] = { + UINT64_C(0x0000000000000000), UINT64_C(0x3ff0000000000000), + UINT64_C(0xbff0000000000000), UINT64_C(0x40c3880000000000), + UINT64_C(0xc0c3880000000000), UINT64_C(0x43abc16d674ec800), + UINT64_C(0x7ff0000000000000), UINT64_C(0xfff0000000000000), + UINT64_C(0x7ff8000000000000), UINT64_C(0x3ff0000000000000), + UINT64_C(0x0000000000000000), UINT64_C(0x41f0000000000000), + UINT64_C(0xc1cd6f3458800000), UINT64_C(0x430c6bf526340000), + UINT64_C(0x3ff0000000000000), UINT64_C(0x0000000000000000), +}; + +// maxPointNumber used by the C++ FloatTS2DIFFEncoder/DoubleTS2DIFFEncoder. +constexpr int kTs2DiffMaxPointNumber = 2; +constexpr double kTs2DiffMaxPointValue = 100.0; + const int32_t kDateValues[] = { 19700101, 19991231, 20000229, 20240229, 20380119, 20500615, 19690720, 19800106, @@ -292,6 +334,12 @@ uint32_t FloatBits(float value) { return bits; } +float BitsToFloat(uint32_t bits) { + float value; + std::memcpy(&value, &bits, sizeof(value)); + return value; +} + float FloatValue(int row) { uint32_t bits = kFloatBits[row % (sizeof(kFloatBits) / sizeof(kFloatBits[0]))]; @@ -300,12 +348,26 @@ float FloatValue(int row) { return value; } +float Ts2DiffFloatValue(int row) { + uint32_t bits = kTs2DiffFloatBits[row % (sizeof(kTs2DiffFloatBits) / + sizeof(kTs2DiffFloatBits[0]))]; + float value; + std::memcpy(&value, &bits, sizeof(value)); + return value; +} + uint64_t DoubleBits(double value) { uint64_t bits; std::memcpy(&bits, &value, sizeof(bits)); return bits; } +double BitsToDouble(uint64_t bits) { + double value; + std::memcpy(&value, &bits, sizeof(value)); + return value; +} + double DoubleValue(int row) { uint64_t bits = kDoubleBits[row % (sizeof(kDoubleBits) / sizeof(kDoubleBits[0]))]; @@ -314,6 +376,76 @@ double DoubleValue(int row) { return value; } +double Ts2DiffDoubleValue(int row) { + uint64_t bits = kTs2DiffDoubleBits[row % (sizeof(kTs2DiffDoubleBits) / + sizeof(kTs2DiffDoubleBits[0]))]; + double value; + std::memcpy(&value, &bits, sizeof(value)); + return value; +} + +bool IsTs2DiffFloatCase(const FixtureCase& fixture_case) { + return fixture_case.encoding == TS_2DIFF && + (fixture_case.data_type == FLOAT || + fixture_case.data_type == DOUBLE); +} + +float ExpectedFloatValue(const FixtureCase& fixture_case, int row) { + if (!IsTs2DiffFloatCase(fixture_case)) { + return FloatValue(row); + } + float value = Ts2DiffFloatValue(row); + if (std::isnan(value)) { + // Java FloatEncoder normalizes any NaN to the canonical pattern + // via floatToIntBits. + return BitsToFloat(0x7fc00000U); + } + // Mirror FloatTS2DIFFEncoder::convert_float_to_int at mpn = 2, then + // FloatDecoder::readFloat: scaled -> stored / 100, scale-overflow -> + // stored / 1, raw bits -> intBitsToFloat. + const double mpv = kTs2DiffMaxPointValue; + const double scaled = static_cast(value) * mpv; + const bool scaled_overflow = + scaled > static_cast(std::numeric_limits::max()) || + scaled < static_cast(std::numeric_limits::min()); + if (scaled_overflow) { + const bool value_overflows = + value > static_cast(std::numeric_limits::max()) || + value < static_cast(std::numeric_limits::min()); + if (value_overflows) { + // Raw IEEE bits, restored losslessly (infinite values here). + return value; + } + return static_cast(std::lround(value)) / 1.0f; + } + return static_cast(std::lround(scaled) / mpv); +} + +double ExpectedDoubleValue(const FixtureCase& fixture_case, int row) { + if (!IsTs2DiffFloatCase(fixture_case)) { + return DoubleValue(row); + } + double value = Ts2DiffDoubleValue(row); + if (std::isnan(value)) { + return BitsToDouble(UINT64_C(0x7ff8000000000000)); + } + const double mpv = kTs2DiffMaxPointValue; + const double scaled = value * mpv; + const bool scaled_overflow = + scaled > static_cast(std::numeric_limits::max()) || + scaled < static_cast(std::numeric_limits::min()); + if (scaled_overflow) { + const bool value_overflows = + value > static_cast(std::numeric_limits::max()) || + value < static_cast(std::numeric_limits::min()); + if (value_overflows) { + return value; + } + return static_cast(std::llround(value)) / 1.0; + } + return std::llround(scaled) / mpv; +} + int32_t DateValue(int row) { return kDateValues[row % (sizeof(kDateValues) / sizeof(kDateValues[0]))]; } @@ -326,6 +458,12 @@ std::vector BuildMatrix() { for (TSDataType data_type : data_types) { cases.emplace_back(data_type, CHIMP, compression, kRowCount); cases.emplace_back(data_type, RLBE, compression, kRowCount); + if (data_type == FLOAT || data_type == DOUBLE) { + // The value set exercises scaled, scale-overflow, and + // raw-bit forms, including the page-wide bitmaps across + // the 129-value TS_2DIFF block boundary. + cases.emplace_back(data_type, TS_2DIFF, compression, kRowCount); + } } cases.emplace_back(DOUBLE, CAMEL, compression, kRowCount); } @@ -346,8 +484,8 @@ TableSchema* CreateTableSchema(const FixtureCase& fixture_case) { column_categories); } -void AddValue(Tablet& tablet, TSDataType data_type, int row) { - switch (data_type) { +void AddValue(Tablet& tablet, const FixtureCase& fixture_case, int row) { + switch (fixture_case.data_type) { case INT32: ASSERT_EQ(E_OK, tablet.add_value(row, kValueColumn, IntValue(row))); break; @@ -362,15 +500,17 @@ void AddValue(Tablet& tablet, TSDataType data_type, int row) { break; case FLOAT: ASSERT_EQ(E_OK, - tablet.add_value(row, kValueColumn, FloatValue(row))); + tablet.add_value(row, kValueColumn, + ExpectedFloatValue(fixture_case, row))); break; case DOUBLE: ASSERT_EQ(E_OK, - tablet.add_value(row, kValueColumn, DoubleValue(row))); + tablet.add_value(row, kValueColumn, + ExpectedDoubleValue(fixture_case, row))); break; default: FAIL() << "Unsupported data type: " - << get_data_type_name(data_type); + << get_data_type_name(fixture_case.data_type); } } @@ -383,7 +523,7 @@ Tablet CreateTablet(TableSchema* table_schema, for (int row = 0; row < fixture_case.row_count; ++row) { EXPECT_EQ(E_OK, tablet.add_timestamp(row, row)); EXPECT_EQ(E_OK, tablet.add_value(row, kTagColumn, kTagValue)); - AddValue(tablet, fixture_case.data_type, row); + AddValue(tablet, fixture_case, row); } return tablet; } @@ -453,11 +593,11 @@ void AssertValue(const FixtureCase& fixture_case, int row, ASSERT_EQ(LongValue(row), result_set->get_value(3)); break; case FLOAT: - ASSERT_EQ(FloatBits(FloatValue(row)), + ASSERT_EQ(FloatBits(ExpectedFloatValue(fixture_case, row)), FloatBits(result_set->get_value(3))); break; case DOUBLE: - ASSERT_EQ(DoubleBits(DoubleValue(row)), + ASSERT_EQ(DoubleBits(ExpectedDoubleValue(fixture_case, row)), DoubleBits(result_set->get_value(3))); break; default: diff --git a/java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java b/java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java index 83025c26f..b2afa3567 100644 --- a/java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java +++ b/java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java @@ -64,7 +64,10 @@ public class TableModelEncodingCompressionCompatibilityTest { private static final String TAG_COLUMN = "device"; private static final String VALUE_COLUMN = "value"; private static final String TAG_VALUE = "compat_device"; - private static final int ROW_COUNT = 32; + // Crosses the 129-value TS_2DIFF block boundary (300 -> 129+129+42), so + // page-wide bitmaps must survive block transitions. Applied to every + // case, per review feedback on apache/tsfile#901. + private static final int ROW_COUNT = 300; private static final int[] INT_VALUES = { 0, @@ -142,6 +145,42 @@ public class TableModelEncodingCompressionCompatibilityTest { 0xc05edd2f1a9fbe77L }; + // FLOAT/DOUBLE + TS_2DIFF converts values to fixed-point with + // maxPointNumber (10^mpn). The expected value is computed by applying the + // same tri-state conversion the writer performs and then decoding it back, + // so expectations reflect the encoding rules rather than the raw input + // bits. Every chosen value has the same expected value whether the writer + // used maxPointNumber 0 (the Java Ts2Diff builder default) or 2 (the + // historical C++ default), so the validating reader never needs to know + // which writer produced the file. The values cover all page layouts the + // writers can produce: + // - plain integers (scaled form; at mpn = 0 every finite in-range value + // is scaled, so pages written by the Java builder contain no bitmap) + // - 1.5e9f / 1e18: the scaled product overflows while round(v) still + // fits -> scale-overflow form, single page-wide bitmap (only reachable + // at mpn > 0, i.e. the C++ writer) + // - NaN and +/-Infinity (stored as raw IEEE bits -> two page-wide + // bitmaps; NaN uses the canonical Java floatToIntBits pattern) + private static final int[] TS_2DIFF_FLOAT_BITS = { + 0x00000000, 0x3f800000, 0xbf800000, 0x461c4000, 0xc61c4000, 0x4eb2d05e, + 0x7f800000, 0xff800000, 0x7fc00000, 0x3f800000, 0x00000000, 0x4a742400, + 0xca742400, 0x4e6e6b28, 0x3f800000, 0x00000000 + }; + + private static final long[] TS_2DIFF_DOUBLE_BITS = { + 0x0000000000000000L, 0x3ff0000000000000L, 0xbff0000000000000L, + 0x40c3880000000000L, 0xc0c3880000000000L, 0x43abc16d674ec800L, + 0x7ff0000000000000L, 0xfff0000000000000L, 0x7ff8000000000000L, + 0x3ff0000000000000L, 0x0000000000000000L, 0x41f0000000000000L, + 0xc1cd6f3458800000L, 0x430c6bf526340000L, 0x3ff0000000000000L, + 0x0000000000000000L + }; + + // maxPointNumber used by the Java Ts2Diff TSEncodingBuilder. + private static final int TS_2DIFF_MAX_POINT_NUMBER = 0; + private static final double TS_2DIFF_MAX_POINT_VALUE = + TS_2DIFF_MAX_POINT_NUMBER <= 0 ? 1.0 : Math.pow(10, TS_2DIFF_MAX_POINT_NUMBER); + private static final LocalDate[] DATE_VALUES = { LocalDate.of(1970, 1, 1), LocalDate.of(1999, 12, 31), @@ -196,6 +235,12 @@ private static List buildMatrix() { TSDataType.DOUBLE)) { cases.add(new FixtureCase(dataType, TSEncoding.CHIMP, compression, ROW_COUNT)); cases.add(new FixtureCase(dataType, TSEncoding.RLBE, compression, ROW_COUNT)); + if (dataType == TSDataType.FLOAT || dataType == TSDataType.DOUBLE) { + // The value set exercises scaled, scale-overflow, and raw-bit + // forms, including the page-wide bitmaps across the 129-value + // TS_2DIFF block boundary. + cases.add(new FixtureCase(dataType, TSEncoding.TS_2DIFF, compression, ROW_COUNT)); + } } cases.add(new FixtureCase(TSDataType.DOUBLE, TSEncoding.CAMEL, compression, ROW_COUNT)); } @@ -270,12 +315,13 @@ private static Tablet tablet(TableSchema tableSchema, FixtureCase fixtureCase) { for (int row = 0; row < fixtureCase.rowCount; row++) { tablet.addTimestamp(row, row); tablet.addValue(TAG_COLUMN, row, TAG_VALUE); - addValue(tablet, fixtureCase.dataType, row); + addValue(tablet, fixtureCase, row); } return tablet; } - private static void addValue(Tablet tablet, TSDataType dataType, int row) { + private static void addValue(Tablet tablet, FixtureCase fixtureCase, int row) { + TSDataType dataType = fixtureCase.dataType; switch (dataType) { case INT32: tablet.addValue(row, VALUE_COLUMN, intValue(row)); @@ -288,10 +334,10 @@ private static void addValue(Tablet tablet, TSDataType dataType, int row) { tablet.addValue(row, VALUE_COLUMN, longValue(row)); break; case FLOAT: - tablet.addValue(row, VALUE_COLUMN, floatValue(row)); + tablet.addValue(row, VALUE_COLUMN, expectedFloatValue(fixtureCase, row)); break; case DOUBLE: - tablet.addValue(row, VALUE_COLUMN, doubleValue(row)); + tablet.addValue(row, VALUE_COLUMN, expectedDoubleValue(fixtureCase, row)); break; default: throw new IllegalArgumentException("Unsupported data type: " + dataType); @@ -313,13 +359,13 @@ private static void assertValue(FixtureCase fixtureCase, int row, ResultSet resu case FLOAT: assertEquals( "FLOAT bits at row " + row, - Float.floatToIntBits(floatValue(row)), + Float.floatToIntBits(expectedFloatValue(fixtureCase, row)), Float.floatToIntBits(resultSet.getFloat(3))); break; case DOUBLE: assertEquals( "DOUBLE bits at row " + row, - Double.doubleToLongBits(doubleValue(row)), + Double.doubleToLongBits(expectedDoubleValue(fixtureCase, row)), Double.doubleToLongBits(resultSet.getDouble(3))); break; default: @@ -343,6 +389,58 @@ private static double doubleValue(int row) { return Double.longBitsToDouble(DOUBLE_BITS[row % DOUBLE_BITS.length]); } + private static boolean isTs2DiffFloatCase(FixtureCase fixtureCase) { + return fixtureCase.encoding == TSEncoding.TS_2DIFF + && (fixtureCase.dataType == TSDataType.FLOAT + || fixtureCase.dataType == TSDataType.DOUBLE); + } + + // Mirror FloatEncoder/FloatDecoder at the Java writer's maxPointNumber: + // scaled -> round(v * mpv) / mpv, scale-overflow -> round(v) / 1, + // raw bits -> intBitsToFloat (NaN is canonicalized by floatToIntBits). + private static float expectedFloatValue(FixtureCase fixtureCase, int row) { + if (!isTs2DiffFloatCase(fixtureCase)) { + return floatValue(row); + } + float value = + Float.intBitsToFloat(TS_2DIFF_FLOAT_BITS[row % TS_2DIFF_FLOAT_BITS.length]); + if (Float.isNaN(value)) { + return Float.intBitsToFloat(0x7fc00000); + } + double mpv = TS_2DIFF_MAX_POINT_VALUE; + double scaled = (double) value * mpv; + if (scaled > Integer.MAX_VALUE || scaled < Integer.MIN_VALUE) { + // value itself stays in int range for the values used here + // (Infinity is caught below), so this is the scale-overflow form. + if (value > Integer.MAX_VALUE || value < Integer.MIN_VALUE) { + return Float.intBitsToFloat(Float.floatToIntBits(value)); + } + return (float) ((double) Math.round(value) / 1.0); + } + return (float) ((double) Math.round(scaled) / mpv); + } + + private static double expectedDoubleValue(FixtureCase fixtureCase, int row) { + if (!isTs2DiffFloatCase(fixtureCase)) { + return doubleValue(row); + } + double value = + Double.longBitsToDouble( + TS_2DIFF_DOUBLE_BITS[row % TS_2DIFF_DOUBLE_BITS.length]); + if (Double.isNaN(value)) { + return Double.longBitsToDouble(0x7ff8000000000000L); + } + double mpv = TS_2DIFF_MAX_POINT_VALUE; + double scaled = value * mpv; + if (scaled > Long.MAX_VALUE || scaled < Long.MIN_VALUE) { + if (value > Long.MAX_VALUE || value < Long.MIN_VALUE) { + return Double.longBitsToDouble(Double.doubleToLongBits(value)); + } + return (double) Math.round(value) / 1.0; + } + return (double) Math.round(scaled) / mpv; + } + private static LocalDate dateValue(int row) { return DATE_VALUES[row % DATE_VALUES.length]; } From 1c5be339d8a3ea0b627c12c9ce6b47f752bca93d Mon Sep 17 00:00:00 2001 From: gx Date: Wed, 26 Aug 2026 12:29:17 +0800 Subject: [PATCH 10/13] fix(cpp): emit page-wide TS_2DIFF float metadata and per-delta bit width Align the C++ FLOAT/DOUBLE TS_2DIFF page layout with the Java canonical format (apache/tsfile#901 review): - The integer encoder's automatic 129-value block flush now emits a plain integer block into an internal page buffer; overflow flags and buffered blocks survive across the boundary. The page-seal flush emits the page metadata once ([overflow marker][pageValueCount] [page-wide bitmap(s)][maxPointNumber]) followed by all buffered blocks, replacing the per-block wrapper metadata. - Bit width is now the maximum width over the raw deltas rebased by min, mirroring Java calculateBitWidthsForDeltaBlockBuffer, instead of the width of (max - min). When raw deltas wrap the signed type (e.g. adjacent raw IEEE bit patterns), (max - min) wrapped negative and the block was silently written with bit width 0, discarding every delta. - The integer flush now calls the base reset() explicitly so the float wrapper's page-scoped state is not cleared by the virtual dispatch during mid-page block flushes. Verified: Java reads all 30 non-LZMA2 C++ fixtures (including FLOAT/DOUBLE TS_2DIFF at 300 rows across the block boundary with NaN/Infinity raw-bit and scale-overflow pages) bit-exactly; the C++ Java-hex golden tests pass. The 15 remaining generate failures are the LZMA2 compression path failing on this MSVC Debug build regardless of encoding (also reproducible with CHIMP + LZMA2 on develop), tracked separately. --- cpp/src/encoding/ts2diff_encoder.h | 249 ++++++++++-------- cpp/test/encoding/ts2diff_codec_test.cc | 8 +- ...lEncodingCompressionCompatibilityTest.java | 10 +- 3 files changed, 141 insertions(+), 126 deletions(-) diff --git a/cpp/src/encoding/ts2diff_encoder.h b/cpp/src/encoding/ts2diff_encoder.h index a39ed6b5f..133be2569 100644 --- a/cpp/src/encoding/ts2diff_encoder.h +++ b/cpp/src/encoding/ts2diff_encoder.h @@ -24,6 +24,7 @@ #include #include +#include #include #include "common/allocator/alloc_base.h" @@ -166,6 +167,32 @@ class TS2DIFFEncoder : public Encoder { return bit_width; } + // Java-compatible bit width: the maximum width over the *rebased* + // deltas, not the width of (max - min). When the raw deltas wrap + // (e.g. two adjacent raw IEEE bit patterns whose difference + // overflows the integer type), (max - min) itself wraps to a + // negative value and cal_bit_width would return 0, silently + // discarding every delta in the block. Mirrors Java + // calculateBitWidthsForDeltaBlockBuffer, which widens each rebased + // delta individually. + int cal_bit_width_rebased() { + typedef typename std::make_unsigned::type UT; + UT max_rebased = 0; + for (int i = 0; i < write_index_; i++) { + UT rebased = static_cast(delta_arr_[i]) - + static_cast(delta_arr_min_); + if (rebased > max_rebased) { + max_rebased = rebased; + } + } + int bit_width = 0; + while (max_rebased > 0) { + bit_width++; + max_rebased >>= 1; + } + return bit_width; + } + // Batch bit-pack `count` values (each `bit_width` bits, MSB-first within // byte) into a single contiguous buffer and write it to out_stream in one // call. Avoids the per-byte write_buf overhead of the scalar write_bits @@ -284,10 +311,12 @@ inline int TS2DIFFEncoder::flush(common::ByteStream& out_stream) { if (write_index_ == -1) { return common::E_OK; } + // Bit width over the raw deltas rebased by min (computed before the + // array itself is rebased, so cal_bit_width_rebased subtracts min + // exactly once). + int bit_width = cal_bit_width_rebased(); // Subtract the minimum value for each delta_arr_ item SIMDOps::rebase(delta_arr_, delta_arr_min_, write_index_); - // Calculate the bit length of each value to writer - int bit_width = cal_bit_width(delta_arr_max_ - delta_arr_min_); // Header writes can fail too (back-pressure / OOM on the underlying // stream); a half-written header followed by reset() leaves the page // corrupted but the caller thinking the data was flushed. @@ -321,7 +350,10 @@ inline int TS2DIFFEncoder::flush(common::ByteStream& out_stream) { // layer can detect the page is poisoned. return pack_ret; } - reset(); + // Base reset only (write_index_ = -1). This is a virtual call so a + // float wrapper encoder can keep its page-scoped state (overflow + // flags, buffered blocks) alive across the per-block flush. + TS2DIFFEncoder::reset(); return ret; } @@ -331,10 +363,11 @@ inline int TS2DIFFEncoder::flush(common::ByteStream& out_stream) { if (write_index_ == -1) { return common::E_OK; } + // Bit width over the raw deltas rebased by min; see the int32 + // specialization for ordering rationale. + int bit_width = cal_bit_width_rebased(); // Subtract the minimum value for each delta_arr_ item SIMDOps::rebase(delta_arr_, delta_arr_min_, write_index_); - // Calculate the bit length of each value to writer - int bit_width = cal_bit_width(delta_arr_max_ - delta_arr_min_); // Header writes can fail too — see int32 specialization for rationale. if (RET_FAIL( common::SerializationUtil::write_i32(write_index_, out_stream))) { @@ -363,7 +396,9 @@ inline int TS2DIFFEncoder::flush(common::ByteStream& out_stream) { } else if (pack_ret != common::E_OK) { return pack_ret; } - reset(); // 语义，writeIndex=-1; + // Base reset only — see the int32 specialization for the virtual-call + // rationale. + TS2DIFFEncoder::reset(); return ret; } @@ -553,19 +588,22 @@ int TS2DIFFEncoder::encode_batch(const int64_t* values, uint32_t count, class FloatTS2DIFFEncoder : public TS2DIFFEncoder { public: - FloatTS2DIFFEncoder() : max_point_number_(2), max_point_value_(100.0) {} + FloatTS2DIFFEncoder() + : max_point_number_(2), + max_point_value_(100.0), + page_blocks_(1024, common::MOD_TS2DIFF_OBJ, false) {} int do_encode(float value, common::ByteStream& out_stream) { int32_t value_int = convert_float_to_int(value); return TS2DIFFEncoder::do_encode(value_int, out_stream); } // PageWriter resets the encoder between pages without going through a // successful flush() (e.g. when the prior page was aborted). The base - // reset() only clears write_index_; underflow_flags_ would otherwise - // leak the prior page's overflow markers into the next page's bitmap. + // reset() only clears write_index_; underflow_flags_ and the buffered + // complete blocks would otherwise leak into the next page. void reset() override { TS2DIFFEncoder::reset(); underflow_flags_.clear(); - max_point_number_saved_ = false; + page_blocks_.reset(); } int flush(common::ByteStream& out_stream) override; int encode(bool value, common::ByteStream& out_stream); @@ -610,26 +648,31 @@ class FloatTS2DIFFEncoder : public TS2DIFFEncoder { int max_point_number_; double max_point_value_; std::vector underflow_flags_; - // Java FloatDecoder reads maxPointNumber once per page; this flag - // makes sure only the first 128-value segment of a page carries the - // prefix. PageWriter/ValuePageWriter reset() between pages clears it, - // so every page starts with a fresh prefix (apache/tsfile#910). - bool max_point_number_saved_{false}; + // Java FloatEncoder emits the page metadata (maxPointNumber and, when + // needed, the page-wide overflow bitmaps) once per page, followed by a + // continuous sequence of integer TS_2DIFF blocks. The integer encoder + // flushes a complete 129-value block on its own whenever it fills, so + // those blocks are buffered here and emitted, metadata first, when the + // page is sealed (apache/tsfile#901 review). + common::ByteStream page_blocks_; }; class DoubleTS2DIFFEncoder : public TS2DIFFEncoder { public: - DoubleTS2DIFFEncoder() : max_point_number_(2), max_point_value_(100.0) {} + DoubleTS2DIFFEncoder() + : max_point_number_(2), + max_point_value_(100.0), + page_blocks_(1024, common::MOD_TS2DIFF_OBJ, false) {} int do_encode(double value, common::ByteStream& out_stream) { int64_t value_long = convert_double_to_long(value); return TS2DIFFEncoder::do_encode(value_long, out_stream); } // See FloatTS2DIFFEncoder::reset for rationale — the prior page's - // overflow markers must not bleed into the next. + // overflow markers and buffered blocks must not bleed into the next. void reset() override { TS2DIFFEncoder::reset(); underflow_flags_.clear(); - max_point_number_saved_ = false; + page_blocks_.reset(); } int flush(common::ByteStream& out_stream) override; int encode(bool value, common::ByteStream& out_stream); @@ -674,11 +717,9 @@ class DoubleTS2DIFFEncoder : public TS2DIFFEncoder { int max_point_number_; double max_point_value_; std::vector underflow_flags_; - // Java FloatDecoder reads maxPointNumber once per page; this flag - // makes sure only the first 128-value segment of a page carries the - // prefix. PageWriter/ValuePageWriter reset() between pages clears it, - // so every page starts with a fresh prefix (apache/tsfile#910). - bool max_point_number_saved_{false}; + // See FloatTS2DIFFEncoder::page_blocks_ — buffered complete integer + // blocks, emitted with the page metadata when the page is sealed. + common::ByteStream page_blocks_; }; typedef TS2DIFFEncoder IntTS2DIFFEncoder; @@ -788,61 +829,49 @@ FORCE_INLINE int DoubleTS2DIFFEncoder::encode(double value, return do_encode(value, out); } -// Keep float/double TS_2DIFF page layout compatible with Java. +// Java FloatEncoder page layout (apache/tsfile#901 review): +// no overflow: [maxPointNumber varint][block 1][block 2]... +// overflow: [overflow marker varint][pageValueCount varint] +// [page-wide bitmap(s)][maxPointNumber varint][blocks...] +// The integer encoder triggers flush() whenever a 129-value block fills +// (write_index_ == block_size_, reachable only from do_encode); those +// blocks are buffered in page_blocks_ without any float metadata. When +// the page is sealed (write_index_ < block_size_), the trailing block is +// appended and the page metadata plus all buffered blocks are emitted. FORCE_INLINE int FloatTS2DIFFEncoder::flush(common::ByteStream& out_stream) { int ret = common::E_OK; - if (write_index_ == -1) { - return common::E_OK; - } - const int num_values = write_index_ + 1; - common::ByteStream inner(1024, common::MOD_TS2DIFF_OBJ, false); - // Java FloatDecoder reads maxPointNumber only once per page; emit it - // just for the page's first segment (apache/tsfile#910). - if (!max_point_number_saved_) { - if (RET_FAIL(common::SerializationUtil::write_var_uint( - static_cast(max_point_number_), inner))) { + const bool block_flush = (write_index_ == block_size_); + if (block_flush) { + // Complete-block flush from do_encode: plain integer block, no + // float wrapper metadata, overflow flags must survive for the + // page-wide bitmap. + return TS2DIFFEncoder::flush(page_blocks_); + } + if (write_index_ != -1) { + // Page-seal flush: append the trailing (partial) integer block. + if (RET_FAIL(TS2DIFFEncoder::flush(page_blocks_))) { return ret; } - max_point_number_saved_ = true; - } - SIMDOps::rebase(delta_arr_, delta_arr_min_, write_index_); - int bit_width = cal_bit_width(delta_arr_max_ - delta_arr_min_); - if (RET_FAIL(common::SerializationUtil::write_ui32( - static_cast(write_index_), inner))) { - return ret; - } - if (RET_FAIL(common::SerializationUtil::write_ui32( - static_cast(bit_width), inner))) { - return ret; - } - if (RET_FAIL(common::SerializationUtil::write_ui32( - static_cast(delta_arr_min_), inner))) { - return ret; - } - if (RET_FAIL(common::SerializationUtil::write_ui32( - static_cast(first_value_), inner))) { - return ret; } - for (int i = 0; i < write_index_; i++) { - write_bits(delta_arr_[i], bit_width, inner); + if (underflow_flags_.empty()) { + // Empty page (nothing encoded, no blocks buffered). + return common::E_OK; } - flush_remaining(inner); - - const bool overflow = has_overflow(); - if (overflow) { - std::vector underflow_bitmap( + const int num_values = static_cast(underflow_flags_.size()); + if (has_overflow()) { + std::vector scaled_bitmap( static_cast(num_values / 8 + 1), 0); - std::vector overflow_bitmap( + std::vector raw_bits_bitmap( static_cast(num_values / 8 + 1), 0); - bool has_original_value_overflow = false; + bool has_raw_bits = false; for (int i = 0; i < num_values; i++) { int8_t f = underflow_flags_[static_cast(i)]; if (f == 1) { - underflow_bitmap[static_cast(i / 8)] |= + scaled_bitmap[static_cast(i / 8)] |= static_cast(1u << (i % 8)); } else if (f == -1) { - has_original_value_overflow = true; - overflow_bitmap[static_cast(i / 8)] |= + has_raw_bits = true; + raw_bits_bitmap[static_cast(i / 8)] |= static_cast(1u << (i % 8)); } } @@ -851,8 +880,8 @@ FORCE_INLINE int FloatTS2DIFFEncoder::flush(common::ByteStream& out_stream) { constexpr uint32_t FLAG_ORIGINAL_VALUE_OVERFLOW = 2147483646u; // Integer.MAX_VALUE - 1 if (RET_FAIL(common::SerializationUtil::write_var_uint( - has_original_value_overflow ? FLAG_ORIGINAL_VALUE_OVERFLOW - : FLAG_SCALED_VALUE_OVERFLOW, + has_raw_bits ? FLAG_ORIGINAL_VALUE_OVERFLOW + : FLAG_SCALED_VALUE_OVERFLOW, out_stream))) { return ret; } @@ -861,15 +890,21 @@ FORCE_INLINE int FloatTS2DIFFEncoder::flush(common::ByteStream& out_stream) { return ret; } const uint32_t bm_len = static_cast(num_values / 8 + 1); - if (RET_FAIL(out_stream.write_buf(underflow_bitmap.data(), bm_len))) { + if (RET_FAIL(out_stream.write_buf(scaled_bitmap.data(), bm_len))) { return ret; } - if (has_original_value_overflow && - RET_FAIL(out_stream.write_buf(overflow_bitmap.data(), bm_len))) { + if (has_raw_bits && + RET_FAIL(out_stream.write_buf(raw_bits_bitmap.data(), bm_len))) { return ret; } } - if (RET_FAIL(merge_byte_stream(out_stream, inner, true))) { + // maxPointNumber sits right before the first integer block in every + // page layout. + if (RET_FAIL(common::SerializationUtil::write_var_uint( + static_cast(max_point_number_), out_stream))) { + return ret; + } + if (RET_FAIL(common::merge_byte_stream(out_stream, page_blocks_))) { return ret; } // Defer encoder-state wipe until after every write into out_stream has @@ -877,60 +912,40 @@ FORCE_INLINE int FloatTS2DIFFEncoder::flush(common::ByteStream& out_stream) { // write_index_ at -1, so the next flush() short-circuited at the top // and the data was silently lost. underflow_flags_.clear(); - TS2DIFFEncoder::reset(); + page_blocks_.reset(); return ret; } +// See FloatTS2DIFFEncoder::flush for the page layout rationale. FORCE_INLINE int DoubleTS2DIFFEncoder::flush(common::ByteStream& out_stream) { int ret = common::E_OK; - if (write_index_ == -1) { - return common::E_OK; + const bool block_flush = (write_index_ == block_size_); + if (block_flush) { + return TS2DIFFEncoder::flush(page_blocks_); } - const int num_values = write_index_ + 1; - common::ByteStream inner(1024, common::MOD_TS2DIFF_OBJ, false); - // Java FloatDecoder reads maxPointNumber only once per page; emit it - // just for the page's first segment (apache/tsfile#910). - if (!max_point_number_saved_) { - if (RET_FAIL(common::SerializationUtil::write_var_uint( - static_cast(max_point_number_), inner))) { + if (write_index_ != -1) { + if (RET_FAIL(TS2DIFFEncoder::flush(page_blocks_))) { return ret; } - max_point_number_saved_ = true; - } - SIMDOps::rebase(delta_arr_, delta_arr_min_, write_index_); - int bit_width = cal_bit_width(delta_arr_max_ - delta_arr_min_); - if (RET_FAIL(common::SerializationUtil::write_i32(write_index_, inner))) { - return ret; } - if (RET_FAIL(common::SerializationUtil::write_i32(bit_width, inner))) { - return ret; - } - if (RET_FAIL(common::SerializationUtil::write_i64(delta_arr_min_, inner))) { - return ret; - } - if (RET_FAIL(common::SerializationUtil::write_i64(first_value_, inner))) { - return ret; - } - for (int i = 0; i < write_index_; i++) { - write_bits(delta_arr_[i], bit_width, inner); + if (underflow_flags_.empty()) { + return common::E_OK; } - flush_remaining(inner); - - const bool overflow = has_overflow(); - if (overflow) { - std::vector underflow_bitmap( + const int num_values = static_cast(underflow_flags_.size()); + if (has_overflow()) { + std::vector scaled_bitmap( static_cast(num_values / 8 + 1), 0); - std::vector overflow_bitmap( + std::vector raw_bits_bitmap( static_cast(num_values / 8 + 1), 0); - bool has_original_value_overflow = false; + bool has_raw_bits = false; for (int i = 0; i < num_values; i++) { int8_t f = underflow_flags_[static_cast(i)]; if (f == 1) { - underflow_bitmap[static_cast(i / 8)] |= + scaled_bitmap[static_cast(i / 8)] |= static_cast(1u << (i % 8)); } else if (f == -1) { - has_original_value_overflow = true; - overflow_bitmap[static_cast(i / 8)] |= + has_raw_bits = true; + raw_bits_bitmap[static_cast(i / 8)] |= static_cast(1u << (i % 8)); } } @@ -939,8 +954,8 @@ FORCE_INLINE int DoubleTS2DIFFEncoder::flush(common::ByteStream& out_stream) { constexpr uint32_t FLAG_ORIGINAL_VALUE_OVERFLOW = 2147483646u; // Integer.MAX_VALUE - 1 if (RET_FAIL(common::SerializationUtil::write_var_uint( - has_original_value_overflow ? FLAG_ORIGINAL_VALUE_OVERFLOW - : FLAG_SCALED_VALUE_OVERFLOW, + has_raw_bits ? FLAG_ORIGINAL_VALUE_OVERFLOW + : FLAG_SCALED_VALUE_OVERFLOW, out_stream))) { return ret; } @@ -949,22 +964,26 @@ FORCE_INLINE int DoubleTS2DIFFEncoder::flush(common::ByteStream& out_stream) { return ret; } const uint32_t bm_len = static_cast(num_values / 8 + 1); - if (RET_FAIL(out_stream.write_buf(underflow_bitmap.data(), bm_len))) { + if (RET_FAIL(out_stream.write_buf(scaled_bitmap.data(), bm_len))) { return ret; } - if (has_original_value_overflow && - RET_FAIL(out_stream.write_buf(overflow_bitmap.data(), bm_len))) { + if (has_raw_bits && + RET_FAIL(out_stream.write_buf(raw_bits_bitmap.data(), bm_len))) { return ret; } } - if (RET_FAIL(merge_byte_stream(out_stream, inner, true))) { + if (RET_FAIL(common::SerializationUtil::write_var_uint( + static_cast(max_point_number_), out_stream))) { + return ret; + } + if (RET_FAIL(common::merge_byte_stream(out_stream, page_blocks_))) { return ret; } // Same deferred-reset rationale as FloatTS2DIFFEncoder::flush — keeping // write_index_ live until every committed write succeeds avoids the // "next flush returns E_OK on lost data" pattern. underflow_flags_.clear(); - TS2DIFFEncoder::reset(); + page_blocks_.reset(); return ret; } diff --git a/cpp/test/encoding/ts2diff_codec_test.cc b/cpp/test/encoding/ts2diff_codec_test.cc index 07fdd94a3..501bdb74d 100644 --- a/cpp/test/encoding/ts2diff_codec_test.cc +++ b/cpp/test/encoding/ts2diff_codec_test.cc @@ -896,9 +896,9 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, } ASSERT_EQ(encoder_float_->flush(out), common::E_OK); - // Byte layout: [FLAG var_uint][n=129][underflow bitmap (17B)] - // [maxPointNumber 0x02][seg1 header][packed] - // [seg2 header starting with 0x00 — no prefix] + // Byte layout: [FLAG var_uint][pageValueCount=140][page-wide + // underflow bitmap (18B)][maxPointNumber 0x02][block 1 header][packed] + // [block 2 header starting with 0x00 — no prefix] std::vector b = byte_stream_bytes(out); size_t pos = 0; uint32_t tag = 0; @@ -906,7 +906,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, EXPECT_EQ(tag, ts2diff_java_detail::FLAG_SCALED_VALUE_OVERFLOW); uint32_t n = 0; ASSERT_TRUE(parse_var_uint(b, pos, n)); - EXPECT_EQ(n, 129u); // segment 1 value count + EXPECT_EQ(n, 140u); // page-wide bitmap covers every value in the page size_t bm_len = static_cast(n / 8 + 1); ASSERT_LE(pos + bm_len, b.size()); pos += bm_len; diff --git a/java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java b/java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java index b2afa3567..e1d32737a 100644 --- a/java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java +++ b/java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java @@ -391,8 +391,7 @@ private static double doubleValue(int row) { private static boolean isTs2DiffFloatCase(FixtureCase fixtureCase) { return fixtureCase.encoding == TSEncoding.TS_2DIFF - && (fixtureCase.dataType == TSDataType.FLOAT - || fixtureCase.dataType == TSDataType.DOUBLE); + && (fixtureCase.dataType == TSDataType.FLOAT || fixtureCase.dataType == TSDataType.DOUBLE); } // Mirror FloatEncoder/FloatDecoder at the Java writer's maxPointNumber: @@ -402,8 +401,7 @@ private static float expectedFloatValue(FixtureCase fixtureCase, int row) { if (!isTs2DiffFloatCase(fixtureCase)) { return floatValue(row); } - float value = - Float.intBitsToFloat(TS_2DIFF_FLOAT_BITS[row % TS_2DIFF_FLOAT_BITS.length]); + float value = Float.intBitsToFloat(TS_2DIFF_FLOAT_BITS[row % TS_2DIFF_FLOAT_BITS.length]); if (Float.isNaN(value)) { return Float.intBitsToFloat(0x7fc00000); } @@ -424,9 +422,7 @@ private static double expectedDoubleValue(FixtureCase fixtureCase, int row) { if (!isTs2DiffFloatCase(fixtureCase)) { return doubleValue(row); } - double value = - Double.longBitsToDouble( - TS_2DIFF_DOUBLE_BITS[row % TS_2DIFF_DOUBLE_BITS.length]); + double value = Double.longBitsToDouble(TS_2DIFF_DOUBLE_BITS[row % TS_2DIFF_DOUBLE_BITS.length]); if (Double.isNaN(value)) { return Double.longBitsToDouble(0x7ff8000000000000L); } From edaf5e84e1880ff7555c2c4718d63c9170764b75 Mon Sep 17 00:00:00 2001 From: gx Date: Wed, 26 Aug 2026 13:28:20 +0800 Subject: [PATCH 11/13] fix(cpp): decode FLOAT/DOUBLE TS_2DIFF pages with page-wide state only Replace the multi-layout sniffing decoder with a single Java-grammar state machine (apache/tsfile#901 review): - Page metadata ([overflow marker][pageValueCount][page-wide bitmap(s)] [maxPointNumber], or bare [maxPointNumber]) is parsed exactly once per page and the bitmaps plus page position survive block transitions, so Java multi-block overflow pages decode correctly. - maxPointNumber = 0 (page starting with 0x00, the Java Ts2Diff builder default) is a valid Form 1 page, no longer misdetected as a legacy raw payload. - The raw bit-cast layout and the pre-#910 per-segment maxPointNumber layout are rejected as format errors instead of being decoded by heuristic detection; legacy tests now assert fail-fast behavior. - Block headers are validated (write_index in [0,128], bit_width in range) and a truncated header now fails instead of silently reusing stale state, which previously let batch readers spin forever on out-of-format input. FLOAT/DOUBLE batch reads reuse the integer batch decoder (SIMD fast path) and apply the page-wide bitmaps afterwards per page position. Verified all four compatibility directions on the extended matrix (30 non-LZMA2 cases each): C++/Java readers on C++/Java writers, including FLOAT/DOUBLE TS_2DIFF at 300 rows across the block boundary with scaled-overflow and raw-bit pages. --- cpp/src/encoding/ts2diff_decoder.h | 617 +++++++++++------------- cpp/test/encoding/ts2diff_codec_test.cc | 133 ++--- 2 files changed, 340 insertions(+), 410 deletions(-) diff --git a/cpp/src/encoding/ts2diff_decoder.h b/cpp/src/encoding/ts2diff_decoder.h index 1175cf4b9..21ee2a77d 100644 --- a/cpp/src/encoding/ts2diff_decoder.h +++ b/cpp/src/encoding/ts2diff_decoder.h @@ -219,217 +219,77 @@ inline bool bitmap_marked(const std::vector& bm, int idx) { return (bm[byte_idx] & static_cast(1u << (idx % 8))) != 0; } -inline bool looks_like_ts2diff_header(common::ByteStream& in) { - int ret = common::E_OK; - uint64_t probe_mark = in.read_pos(); - int32_t write_index = 0; - int32_t bit_width = 0; - if (RET_FAIL(common::SerializationUtil::read_i32(write_index, in)) || - RET_FAIL(common::SerializationUtil::read_i32(bit_width, in))) { - in.set_read_pos(probe_mark); - return false; - } - in.set_read_pos(probe_mark); - if (write_index < 0 || write_index > 128) { - return false; - } - if (bit_width < 0 || bit_width > 64) { - return false; - } - return true; -} - -struct SegmentHeaderPreload { - int32_t write_index = 0; - int32_t bit_width = 0; - int64_t delta_min = 0; - int64_t first_value = 0; - bool ready = false; +// Page-level FLOAT/DOUBLE metadata, parsed exactly once per page. +// Layout (see cpp/docs/ts2diff-float-double-wire-format.md): +// form 1: [maxPointNumber varint] +// form 2: [Integer.MAX_VALUE][count][scaled bitmap][maxPointNumber] +// form 3: [Integer.MAX_VALUE-1][count][scaled bitmap][raw bitmap] +// [maxPointNumber] +// A leading 0x00 byte is the normal encoding of maxPointNumber = 0 (the +// Java Ts2Diff builder default), not a legacy marker. Inputs that do not +// match this grammar are a format error (E_TSFILE_CORRUPTED). +struct PageMeta { + bool has_scaled_bm = false; + bool has_raw_bm = false; + std::vector scaled_bm; + std::vector raw_bm; + int max_point_number = 0; + int page_value_count = 0; }; -// Reads a LEB128 var_uint where the first byte was already consumed into -// `first_byte`. Forward-only: never rewinds the stream. -inline int read_var_uint_tail(uint8_t first_byte, common::ByteStream& in, - uint32_t& out) { - int ret = common::E_OK; - out = static_cast(first_byte & 0x7F); - int shift = 7; - uint8_t b = first_byte; - while (b & 0x80) { - uint32_t read_len = 0; - if (RET_FAIL(in.read_buf(&b, 1, read_len)) || read_len != 1) { - return ret; - } - if (shift > 28) { - return common::E_TSFILE_CORRUPTED; - } - out |= static_cast(b & 0x7F) << shift; - shift += 7; - } - return common::E_OK; -} - -// Parses the segment header (write_index + bit_width + delta_min + -// first_value) forward-only. `wi_hi` is the first (already consumed) byte -// of the big-endian write_index - always 0x00 for the no-prefix layout. -inline int read_segment_header_preload(common::ByteStream& in, bool is_double, - uint8_t wi_hi, SegmentHeaderPreload& h) { +inline int read_page_meta(common::ByteStream& in, PageMeta& meta) { int ret = common::E_OK; - uint8_t rest[3] = {0, 0, 0}; - uint32_t read_len = 0; - if (RET_FAIL(in.read_buf(rest, 3, read_len)) || read_len != 3) { - return ret; - } - h.write_index = (static_cast(wi_hi) << 24) | - (static_cast(rest[0]) << 16) | - (static_cast(rest[1]) << 8) | - static_cast(rest[2]); - int32_t bw = 0; - if (RET_FAIL(common::SerializationUtil::read_i32(bw, in))) { - return ret; - } - h.bit_width = bw; - if (is_double) { - if (RET_FAIL(common::SerializationUtil::read_i64(h.delta_min, in))) { - return ret; - } - if (RET_FAIL(common::SerializationUtil::read_i64(h.first_value, in))) { - return ret; - } - } else { - int32_t dm = 0; - int32_t fv = 0; - if (RET_FAIL(common::SerializationUtil::read_i32(dm, in))) { - return ret; - } - if (RET_FAIL(common::SerializationUtil::read_i32(fv, in))) { - return ret; - } - h.delta_min = dm; - h.first_value = fv; - } - h.ready = true; - return common::E_OK; -} - -inline int consume_float_double_ts2diff_prefix( - common::ByteStream& in, bool& is_legacy_raw, bool& max_pn_present, - int& max_point_number, std::vector& underflow_bm, - std::vector& overflow_bm, int& segment_size, - bool page_first_segment, bool is_double, SegmentHeaderPreload& preload) { - int ret = common::E_OK; - is_legacy_raw = false; - max_pn_present = true; - max_point_number = 0; - underflow_bm.clear(); - overflow_bm.clear(); - segment_size = 0; - uint64_t mark = in.read_pos(); - // apache/tsfile#910 layout: only the page's first segment carries the - // Java maxPointNumber prefix; later segments start directly with the - // 4-byte write_index whose high byte is 0x00. This library always - // serializes max_point_number_ = 2 (0x02), so a leading 0x00 can only - // mean "no prefix on this segment". - // - // Everything is parsed forward-only: rewinding to a page-aligned offset - // (e.g. the start of a page) makes ByteStream::check_space() advance - // the page cursor one page too far and fail the next read, so no - // peek-and-restore is used here. - uint8_t first_byte = 0; - uint32_t read_len = 0; - if (RET_FAIL(in.read_buf(&first_byte, 1, read_len)) || read_len != 1) { - return ret; - } - if (first_byte == 0x00) { - // No prefix: the segment header begins with write_index 0x00... - if (page_first_segment) { - // A page whose very first segment has no prefix is a legacy - // raw C++ block page (no scaling at all). - is_legacy_raw = true; - } - max_pn_present = false; - if (RET_FAIL(read_segment_header_preload(in, is_double, first_byte, - preload))) { - return ret; - } - return common::E_OK; - } uint32_t tag = 0; - if (RET_FAIL(read_var_uint_tail(first_byte, in, tag))) { + if (RET_FAIL(common::SerializationUtil::read_var_uint(tag, in))) { return ret; } - if (tag == FLAG_ORIGINAL_VALUE_OVERFLOW || - tag == FLAG_SCALED_VALUE_OVERFLOW) { - uint32_t n = 0; - if (RET_FAIL(common::SerializationUtil::read_var_uint(n, in))) { + if (tag == FLAG_SCALED_VALUE_OVERFLOW || + tag == FLAG_ORIGINAL_VALUE_OVERFLOW) { + uint32_t count = 0; + if (RET_FAIL(common::SerializationUtil::read_var_uint(count, in))) { return ret; } - segment_size = static_cast(n); - int bm_len = segment_size / 8 + 1; - underflow_bm.resize(static_cast(bm_len), 0); - if (RET_FAIL(in.read_buf(underflow_bm.data(), + if (count == 0 || count > 0x7FFFFFFFu) { + return common::E_TSFILE_CORRUPTED; + } + const int bm_len = static_cast(count) / 8 + 1; + meta.has_scaled_bm = true; + meta.scaled_bm.resize(static_cast(bm_len), 0); + uint32_t read_len = 0; + if (RET_FAIL(in.read_buf(meta.scaled_bm.data(), static_cast(bm_len), read_len)) || read_len != static_cast(bm_len)) { - return ret; + return common::E_TSFILE_CORRUPTED; } if (tag == FLAG_ORIGINAL_VALUE_OVERFLOW) { - overflow_bm.resize(static_cast(bm_len), 0); - if (RET_FAIL(in.read_buf(overflow_bm.data(), + meta.has_raw_bm = true; + meta.raw_bm.resize(static_cast(bm_len), 0); + if (RET_FAIL(in.read_buf(meta.raw_bm.data(), static_cast(bm_len), read_len)) || read_len != static_cast(bm_len)) { - return ret; - } - } - if (page_first_segment) { - // First segment: maxPointNumber always follows the bitmaps. - uint32_t mpn = 0; - if (RET_FAIL(common::SerializationUtil::read_var_uint(mpn, in))) { - return ret; + return common::E_TSFILE_CORRUPTED; } - max_point_number = static_cast(mpn); - return common::E_OK; - } - // Later segment: new-format pages jump straight to the segment - // header (0x00 write_index high byte); old-format pages repeat the - // maxPointNumber prefix here. - uint8_t after_bm_byte = 0; - if (RET_FAIL(in.read_buf(&after_bm_byte, 1, read_len)) || - read_len != 1) { - return ret; - } - if (after_bm_byte == 0x00) { - max_pn_present = false; - if (RET_FAIL(read_segment_header_preload(in, is_double, - after_bm_byte, preload))) { - return ret; - } - return common::E_OK; } + meta.page_value_count = static_cast(count); uint32_t mpn = 0; - if (RET_FAIL(read_var_uint_tail(after_bm_byte, in, mpn))) { + if (RET_FAIL(common::SerializationUtil::read_var_uint(mpn, in))) { return ret; } - max_point_number = static_cast(mpn); - return common::E_OK; - } - - // A non-flag tag is the maxPointNumber prefix itself. - max_point_number = static_cast(tag); - if (!looks_like_ts2diff_header(in)) { - // Only reachable on corrupt/nonstandard data: a non-flag tag whose - // following bytes are not a valid segment header. Rewind and fall - // back to the raw-block path. The rewind target may be page-aligned - // (e.g. a page start), which trips ByteStream::check_space's page - // cursor - accepted here because valid data never takes this branch. - in.set_read_pos(mark); - is_legacy_raw = true; - max_pn_present = false; + if (mpn > 100) { + return common::E_TSFILE_CORRUPTED; + } + meta.max_point_number = static_cast(mpn); } else { - segment_size = 0; + if (tag > 100) { + return common::E_TSFILE_CORRUPTED; + } + meta.max_point_number = static_cast(tag); + meta.page_value_count = 0; // unknown until the blocks are decoded } return common::E_OK; } + } // namespace ts2diff_java_detail // ============================================================================ @@ -452,7 +312,7 @@ class TS2DIFFDecoder : public Decoder { bit_width_ = 0; current_index_ = 0; header_peeked_ = false; - header_preloaded_ = false; + header_error_ = false; } FORCE_INLINE bool has_remaining(const common::ByteStream& buffer) override { @@ -462,9 +322,30 @@ class TS2DIFFDecoder : public Decoder { current_index_ != 0); } - void read_header(common::ByteStream& in) { - common::SerializationUtil::read_i32(write_index_, in); - common::SerializationUtil::read_i32(bit_width_, in); + // Reads the 4+4 byte block header. On a truncated stream the old + // signature silently kept the previous (stale) write_index_, letting a + // caller loop forever re-emitting a phantom block; the failure is + // recorded in header_error_ (decode() returns a value, not an error + // code) and returned for batch entry points. + int read_header(common::ByteStream& in) { + int32_t write_index = 0; + int32_t bit_width = 0; + if (common::SerializationUtil::read_i32(write_index, in) != + common::E_OK || + common::SerializationUtil::read_i32(bit_width, in) != + common::E_OK) { + header_error_ = true; + return common::E_TSFILE_CORRUPTED; + } + if (write_index < 0 || write_index > 128 || bit_width < 0 || + bit_width > (int)sizeof(T) * 8) { + header_error_ = true; + return common::E_TSFILE_CORRUPTED; + } + write_index_ = write_index; + bit_width_ = bit_width; + header_error_ = false; + return common::E_OK; } // If empty, cache 8 bits from in_stream to 'buffer_'. @@ -537,9 +418,10 @@ class TS2DIFFDecoder : public Decoder { int write_index_; int current_index_; bool header_peeked_; - // Set when consume_float_double_ts2diff_prefix already parsed the - // segment header (prefix-free segment); decode() must not re-read it. - bool header_preloaded_{false}; + // Sticky: the last block header failed to parse or was out of range. + // decode() cannot return an error code, so batch entry points check + // this flag to stop instead of looping on phantom blocks. + bool header_error_{false}; }; // ============================================================================ @@ -550,15 +432,15 @@ template <> inline int32_t TS2DIFFDecoder::decode(common::ByteStream& in) { int32_t ret_value = stored_value_; if (UNLIKELY(current_index_ == 0)) { - // A prefix-free segment (no maxPointNumber) has its header parsed - // by consume_float_double_ts2diff_prefix already. - if (UNLIKELY(header_preloaded_)) { - header_preloaded_ = false; - } else { - read_header(in); - common::SerializationUtil::read_i32(delta_min_, in); - common::SerializationUtil::read_i32(first_value_, in); + if (read_header(in) != common::E_OK) { + // Poison the block state so callers stop; value is undefined + // for corrupt input. + write_index_ = 0; + current_index_ = 0; + return ret_value; } + common::SerializationUtil::read_i32(delta_min_, in); + common::SerializationUtil::read_i32(first_value_, in); ret_value = first_value_; bits_left_ = 0; buffer_ = 0; @@ -585,13 +467,13 @@ template <> inline int64_t TS2DIFFDecoder::decode(common::ByteStream& in) { int64_t ret_value = stored_value_; if (UNLIKELY(current_index_ == 0)) { - if (UNLIKELY(header_preloaded_)) { - header_preloaded_ = false; - } else { - read_header(in); - common::SerializationUtil::read_i64(delta_min_, in); - common::SerializationUtil::read_i64(first_value_, in); + if (read_header(in) != common::E_OK) { + write_index_ = 0; + current_index_ = 0; + return ret_value; } + common::SerializationUtil::read_i64(delta_min_, in); + common::SerializationUtil::read_i64(first_value_, in); ret_value = first_value_; if (write_index_ == 0) { current_index_ = 0; @@ -632,7 +514,9 @@ inline int TS2DIFFDecoder::read_batch_int32(int32_t* out, int capacity, } // Start of a new block — read header - read_header(in); + if (read_header(in) != common::E_OK) { + return common::E_TSFILE_CORRUPTED; + } common::SerializationUtil::read_i32(delta_min_, in); common::SerializationUtil::read_i32(first_value_, in); bits_left_ = 0; @@ -748,7 +632,9 @@ inline int TS2DIFFDecoder::read_batch_int64(int64_t* out, int capacity, // Start of a new block if (!header_peeked_) { - read_header(in); + if (read_header(in) != common::E_OK) { + return common::E_TSFILE_CORRUPTED; + } common::SerializationUtil::read_i64(delta_min_, in); common::SerializationUtil::read_i64(first_value_, in); bits_left_ = 0; @@ -985,7 +871,9 @@ inline bool TS2DIFFDecoder::peek_next_block_range_int64( // value decoder (value decoders decode normally and never call this). if (current_index_ != 0 || !has_remaining(in)) return false; - read_header(in); + if (read_header(in) != common::E_OK) { + return common::E_TSFILE_CORRUPTED; + } common::SerializationUtil::read_i64(delta_min_, in); common::SerializationUtil::read_i64(first_value_, in); bits_left_ = 0; @@ -1086,20 +974,17 @@ inline int TS2DIFFDecoder::skip_int32(int count, int& skipped, class FloatTS2DIFFDecoder : public TS2DIFFDecoder { public: FloatTS2DIFFDecoder() = default; - // PageReader invokes reset() at every page boundary; the first segment - // of a page is the only one that may carry the maxPointNumber prefix. + // PageReader invokes reset() at every page boundary; the page-level + // FLOAT/DOUBLE metadata (maxPointNumber, page-wide bitmaps, page value + // position) is parsed once per page and survives block transitions. void reset() override { TS2DIFFDecoder::reset(); - page_first_segment_ = true; - // A legacy raw page sets is_legacy_raw_ for the whole object; clear - // it (and the per-page scale/bitmap state) so a decoder object - // reused across pages stays correct. - is_legacy_raw_ = false; + page_meta_parsed_ = false; max_point_value_ = 1.0; - underflow_bm_.clear(); - overflow_bm_.clear(); - segment_pos_ = 0; - segment_size_ = 0; + page_pos_ = 0; + page_value_count_ = 0; + scaled_bm_.clear(); + raw_bm_.clear(); } float decode(common::ByteStream& in) { int32_t value_int = TS2DIFFDecoder::decode(in); @@ -1113,25 +998,52 @@ class FloatTS2DIFFDecoder : public TS2DIFFDecoder { int read_double(double& ret_value, common::ByteStream& in) override; int read_batch_float(float* out, int capacity, int& actual, - common::ByteStream& in) override { - // FLOAT TS_2DIFF segments have a scale/overflow prefix before the - // integer delta block. The integer batch decoder does not consume - // that prefix, so use the segment-aware scalar decoder here. - // Note: skip_int32/skip_int64 are likewise unsupported on the - // float/double decoders - the segment prefix layout makes the raw - // header-skip path invalid. - return Decoder::read_batch_float(out, capacity, actual, in); - } + common::ByteStream& in) override; + int read_batch_int32(int32_t* out, int capacity, int& actual, + common::ByteStream& in) override; private: - bool is_legacy_raw_{false}; - int max_point_number_{0}; + // Parses the page metadata on the first value of a page. Returns + // E_OK and leaves the stream positioned at the first integer block. + int ensure_page_meta(common::ByteStream& in) { + if (page_meta_parsed_) { + return common::E_OK; + } + ts2diff_java_detail::PageMeta meta; + int ret = ts2diff_java_detail::read_page_meta(in, meta); + if (RET_FAIL(ret)) { + return ret; + } + max_point_value_ = + meta.max_point_number <= 0 + ? 1.0 + : std::pow(10.0, static_cast(meta.max_point_number)); + page_value_count_ = meta.page_value_count; + scaled_bm_ = std::move(meta.scaled_bm); + raw_bm_ = std::move(meta.raw_bm); + page_pos_ = 0; + page_meta_parsed_ = true; + return common::E_OK; + } + + float value_at(int32_t value_int) const { + if (!raw_bm_.empty() && + ts2diff_java_detail::bitmap_marked(raw_bm_, page_pos_)) { + return common::int_to_float(value_int); + } + const bool use_scaled = + scaled_bm_.empty() || + ts2diff_java_detail::bitmap_marked(scaled_bm_, page_pos_); + const double divisor = use_scaled ? max_point_value_ : 1.0; + return static_cast(static_cast(value_int) / divisor); + } + double max_point_value_{1.0}; - int segment_pos_{0}; - int segment_size_{0}; - std::vector underflow_bm_; - std::vector overflow_bm_; - bool page_first_segment_{true}; + int page_pos_{0}; + int page_value_count_{0}; + bool page_meta_parsed_{false}; + std::vector scaled_bm_; + std::vector raw_bm_; }; class DoubleTS2DIFFDecoder : public TS2DIFFDecoder { @@ -1139,13 +1051,12 @@ class DoubleTS2DIFFDecoder : public TS2DIFFDecoder { DoubleTS2DIFFDecoder() = default; void reset() override { TS2DIFFDecoder::reset(); - page_first_segment_ = true; - is_legacy_raw_ = false; + page_meta_parsed_ = false; max_point_value_ = 1.0; - underflow_bm_.clear(); - overflow_bm_.clear(); - segment_pos_ = 0; - segment_size_ = 0; + page_pos_ = 0; + page_value_count_ = 0; + scaled_bm_.clear(); + raw_bm_.clear(); } double decode(common::ByteStream& in) { int64_t value_long = TS2DIFFDecoder::decode(in); @@ -1159,24 +1070,50 @@ class DoubleTS2DIFFDecoder : public TS2DIFFDecoder { int read_double(double& ret_value, common::ByteStream& in) override; int read_batch_double(double* out, int capacity, int& actual, - common::ByteStream& in) override { - // DOUBLE TS_2DIFF uses the same segment prefix. Bypassing - // read_double() misreads that prefix as a block header and can spin - // at end-of-input while decoding an otherwise valid page. - // skip_int32/skip_int64 are likewise unsupported here (see the - // float decoder note). - return Decoder::read_batch_double(out, capacity, actual, in); - } + common::ByteStream& in) override; + int read_batch_int64(int64_t* out, int capacity, int& actual, + common::ByteStream& in) override; private: - bool is_legacy_raw_{false}; - int max_point_number_{0}; + int ensure_page_meta(common::ByteStream& in) { + if (page_meta_parsed_) { + return common::E_OK; + } + ts2diff_java_detail::PageMeta meta; + int ret = ts2diff_java_detail::read_page_meta(in, meta); + if (RET_FAIL(ret)) { + return ret; + } + max_point_value_ = + meta.max_point_number <= 0 + ? 1.0 + : std::pow(10.0, static_cast(meta.max_point_number)); + page_value_count_ = meta.page_value_count; + scaled_bm_ = std::move(meta.scaled_bm); + raw_bm_ = std::move(meta.raw_bm); + page_pos_ = 0; + page_meta_parsed_ = true; + return common::E_OK; + } + + double value_at(int64_t value_long) const { + if (!raw_bm_.empty() && + ts2diff_java_detail::bitmap_marked(raw_bm_, page_pos_)) { + return common::long_to_double(value_long); + } + const bool use_scaled = + scaled_bm_.empty() || + ts2diff_java_detail::bitmap_marked(scaled_bm_, page_pos_); + const double divisor = use_scaled ? max_point_value_ : 1.0; + return static_cast(value_long) / divisor; + } + double max_point_value_{1.0}; - int segment_pos_{0}; - int segment_size_{0}; - std::vector underflow_bm_; - std::vector overflow_bm_; - bool page_first_segment_{true}; + int page_pos_{0}; + int page_value_count_{0}; + bool page_meta_parsed_{false}; + std::vector scaled_bm_; + std::vector raw_bm_; }; typedef TS2DIFFDecoder IntTS2DIFFDecoder; @@ -1275,58 +1212,55 @@ FORCE_INLINE int FloatTS2DIFFDecoder::read_int64(int64_t& ret_value, FORCE_INLINE int FloatTS2DIFFDecoder::read_float(float& ret_value, common::ByteStream& in) { int ret = common::E_OK; - if (current_index_ == 0 && !is_legacy_raw_) { - bool max_pn_present = true; - ts2diff_java_detail::SegmentHeaderPreload preload; - if (RET_FAIL(ts2diff_java_detail::consume_float_double_ts2diff_prefix( - in, is_legacy_raw_, max_pn_present, max_point_number_, - underflow_bm_, overflow_bm_, segment_size_, page_first_segment_, - false, preload))) { + if (RET_FAIL(ensure_page_meta(in))) { + return ret; + } + int32_t value_int = TS2DIFFDecoder::decode(in); + ret_value = value_at(value_int); + page_pos_++; + return common::E_OK; +} +FORCE_INLINE int FloatTS2DIFFDecoder::read_double(double& ret_value, + common::ByteStream& in) { + ASSERT(false); + return common::E_NOT_SUPPORT; +} +// Page-level metadata is parsed once, then the integer blocks are decoded +// with the integer batch decoder (SIMD fast path where available) and the +// page-wide bitmaps are applied afterwards using the page position. +FORCE_INLINE int FloatTS2DIFFDecoder::read_batch_float(float* out, int capacity, + int& actual, + common::ByteStream& in) { + int ret = common::E_OK; + actual = 0; + if (RET_FAIL(ensure_page_meta(in))) { + return ret; + } + constexpr int kIntsPerBatch = 128; + int32_t ints[kIntsPerBatch]; + while (actual < capacity) { + int block_actual = 0; + const int want = capacity - actual; + const int request = want < kIntsPerBatch ? want : kIntsPerBatch; + if (RET_FAIL(TS2DIFFDecoder::read_batch_int32( + ints, request, block_actual, in))) { return ret; } - // maxPointNumber is written once per page; later segments of - // the page reuse the first segment's scale factor. - if (max_pn_present) { - max_point_value_ = - max_point_number_ <= 0 - ? 1.0 - : std::pow(10.0, static_cast(max_point_number_)); + if (block_actual == 0) { + break; } - // Prefix-free segments have their header parsed up front so that - // decode() can pick it up without re-reading the stream. - if (preload.ready) { - write_index_ = preload.write_index; - bit_width_ = preload.bit_width; - delta_min_ = static_cast(preload.delta_min); - first_value_ = static_cast(preload.first_value); - header_preloaded_ = true; + for (int i = 0; i < block_actual; i++) { + out[actual + i] = value_at(ints[i]); + page_pos_++; } - page_first_segment_ = false; - segment_pos_ = 0; + actual += block_actual; } - if (is_legacy_raw_) { - ret_value = decode(in); - return common::E_OK; - } - int32_t value_int = TS2DIFFDecoder::decode(in); - if (!overflow_bm_.empty() && - ts2diff_java_detail::bitmap_marked(overflow_bm_, segment_pos_)) { - ret_value = common::int_to_float(value_int); - } else { - bool use_scaled = true; - if (!underflow_bm_.empty()) { - use_scaled = - ts2diff_java_detail::bitmap_marked(underflow_bm_, segment_pos_); - } - const double divisor = use_scaled ? max_point_value_ : 1.0; - ret_value = - static_cast(static_cast(value_int) / divisor); - } - segment_pos_++; return common::E_OK; } -FORCE_INLINE int FloatTS2DIFFDecoder::read_double(double& ret_value, - common::ByteStream& in) { +FORCE_INLINE int FloatTS2DIFFDecoder::read_batch_int32(int32_t* out, + int capacity, + int& actual, + common::ByteStream& in) { ASSERT(false); return common::E_NOT_SUPPORT; } @@ -1353,53 +1287,48 @@ FORCE_INLINE int DoubleTS2DIFFDecoder::read_float(float& ret_value, FORCE_INLINE int DoubleTS2DIFFDecoder::read_double(double& ret_value, common::ByteStream& in) { int ret = common::E_OK; - if (current_index_ == 0 && !is_legacy_raw_) { - bool max_pn_present = true; - ts2diff_java_detail::SegmentHeaderPreload preload; - if (RET_FAIL(ts2diff_java_detail::consume_float_double_ts2diff_prefix( - in, is_legacy_raw_, max_pn_present, max_point_number_, - underflow_bm_, overflow_bm_, segment_size_, page_first_segment_, - true, preload))) { + if (RET_FAIL(ensure_page_meta(in))) { + return ret; + } + int64_t value_long = TS2DIFFDecoder::decode(in); + ret_value = value_at(value_long); + page_pos_++; + return common::E_OK; +} +// See FloatTS2DIFFDecoder::read_batch_float for the layout rationale. +FORCE_INLINE int DoubleTS2DIFFDecoder::read_batch_double( + double* out, int capacity, int& actual, common::ByteStream& in) { + int ret = common::E_OK; + actual = 0; + if (RET_FAIL(ensure_page_meta(in))) { + return ret; + } + constexpr int kIntsPerBatch = 128; + int64_t ints[kIntsPerBatch]; + while (actual < capacity) { + int block_actual = 0; + const int want = capacity - actual; + const int request = want < kIntsPerBatch ? want : kIntsPerBatch; + if (RET_FAIL(TS2DIFFDecoder::read_batch_int64( + ints, request, block_actual, in))) { return ret; } - // maxPointNumber is written once per page; later segments of - // the page reuse the first segment's scale factor. - if (max_pn_present) { - max_point_value_ = - max_point_number_ <= 0 - ? 1.0 - : std::pow(10.0, static_cast(max_point_number_)); + if (block_actual == 0) { + break; } - if (preload.ready) { - write_index_ = preload.write_index; - bit_width_ = preload.bit_width; - delta_min_ = preload.delta_min; - first_value_ = preload.first_value; - header_preloaded_ = true; + for (int i = 0; i < block_actual; i++) { + out[actual + i] = value_at(ints[i]); + page_pos_++; } - page_first_segment_ = false; - segment_pos_ = 0; + actual += block_actual; } - if (is_legacy_raw_) { - ret_value = decode(in); - return common::E_OK; - } - int64_t value_long = TS2DIFFDecoder::decode(in); - if (!overflow_bm_.empty() && - ts2diff_java_detail::bitmap_marked(overflow_bm_, segment_pos_)) { - ret_value = common::long_to_double(value_long); - } else { - bool use_scaled = true; - if (!underflow_bm_.empty()) { - use_scaled = - ts2diff_java_detail::bitmap_marked(underflow_bm_, segment_pos_); - } - const double divisor = use_scaled ? max_point_value_ : 1.0; - ret_value = static_cast(value_long) / divisor; - } - segment_pos_++; return common::E_OK; } +FORCE_INLINE int DoubleTS2DIFFDecoder::read_batch_int64( + int64_t* out, int capacity, int& actual, common::ByteStream& in) { + ASSERT(false); + return common::E_NOT_SUPPORT; +} } // end namespace storage #endif // ENCODING_TS2DIFF_DECODER_H diff --git a/cpp/test/encoding/ts2diff_codec_test.cc b/cpp/test/encoding/ts2diff_codec_test.cc index 501bdb74d..6716125cf 100644 --- a/cpp/test/encoding/ts2diff_codec_test.cc +++ b/cpp/test/encoding/ts2diff_codec_test.cc @@ -526,17 +526,22 @@ TEST(FloatTS2DIFFEncoderResetTest, ResetClearsUnderflowFlags) { // Regression: legacy raw float/double segments (written by the old C++ // encoders, i.e. plain int delta blocks with no maxPointNumber / overflow // prefix, values stored as bit-cast float bits) must stay decodable through -// read_batch_float / read_batch_double. The per-block prefix heuristic -// used to misclassify a valid raw header as a maxPointNumber prefix, -// desyncing the stream and spinning at end-of-input (PR #901 review). +// Legacy raw TS_2DIFF pages (pre-#796 C++ writer output: plain int delta +// blocks over bit-cast float bits, no maxPointNumber / bitmap prefix) are +// outside the cross-language format: the Java reader never supported them +// (apache/tsfile#901 review). The decoder now treats such input as a +// format error instead of guessing the layout: it must fail fast rather +// than hang at end-of-input or silently return misdecoded values. +// +// A raw block starts with the 4-byte big-endian write_index whose high +// byte is 0x00; to the page-metadata parser that is a valid +// maxPointNumber = 0 (Form 1), so detection happens at the block level: +// the following bytes are not a consistent block stream and the batch +// reader must terminate with an error rather than loop forever. TEST_F(FloatDoubleTS2DIFFCodecTest, ReadBatchFloatLegacyRawSegments) { common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); const int row_num = 129; std::vector expected(row_num); - // 128 equal values followed by a change: the first legacy block has - // bit_width = 0 and delta_min = 0, exactly the pattern the old - // heuristic misclassified. The trailing 1-value block (write_index = 0) - // exercised the same heuristic again. for (int i = 0; i < row_num; ++i) { expected[i] = (i < 128) ? 1.5f : 2.5f; } @@ -549,21 +554,12 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, ReadBatchFloatLegacyRawSegments) { ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); std::vector actual_values(row_num); - int decoded = 0; - // Small batches exercise the layout decision plus repeated block - // transitions. - while (decoded < row_num) { - int actual = 0; - ASSERT_EQ(decoder_float_->read_batch_float( - actual_values.data() + decoded, 16, actual, out_stream), - common::E_OK); - ASSERT_GT(actual, 0); - decoded += actual; - } - for (int i = 0; i < row_num; ++i) { - EXPECT_EQ(actual_values[i], expected[i]) << "row " << i; - } - EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); + int actual = 0; + // Must terminate (no end-of-input spin) and report a format error; + // never return E_OK with garbage values. + const int rc = decoder_float_->read_batch_float( + actual_values.data(), row_num, actual, out_stream); + ASSERT_NE(rc, common::E_OK); } TEST_F(FloatDoubleTS2DIFFCodecTest, ReadBatchDoubleLegacyRawSegments) { @@ -582,24 +578,12 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, ReadBatchDoubleLegacyRawSegments) { ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); std::vector actual_values(row_num); - int decoded = 0; - while (decoded < row_num) { - int actual = 0; - ASSERT_EQ(decoder_double_->read_batch_double( - actual_values.data() + decoded, 16, actual, out_stream), - common::E_OK); - ASSERT_GT(actual, 0); - decoded += actual; - } - for (int i = 0; i < row_num; ++i) { - EXPECT_EQ(actual_values[i], expected[i]) << "row " << i; - } - EXPECT_FALSE(decoder_double_->has_remaining(out_stream)); + int actual = 0; + const int rc = decoder_double_->read_batch_double( + actual_values.data(), row_num, actual, out_stream); + ASSERT_NE(rc, common::E_OK); } -// The layout decision must also cover the scalar path: legacy raw pages -// read one value at a time via read_float / read_double keep the bit-cast -// semantics across the 128-value block boundary. TEST_F(FloatDoubleTS2DIFFCodecTest, ReadFloatLegacyRawScalar) { common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); const int row_num = 129; @@ -616,11 +600,19 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, ReadFloatLegacyRawScalar) { ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); float v = 0.f; + // Scalar path: may return a bounded number of values, but must not + // spin forever; assert that reading the full stream terminates and + // does not report success for all rows of a known-invalid layout. + int ok_rows = 0; + int rc = common::E_OK; for (int i = 0; i < row_num; ++i) { - ASSERT_EQ(decoder_float_->read_float(v, out_stream), common::E_OK); - EXPECT_EQ(v, expected[i]) << "row " << i; + rc = decoder_float_->read_float(v, out_stream); + if (rc != common::E_OK) break; + ok_rows++; } - EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); + // Termination is the contract; the values themselves are undefined + // for this out-of-format input. + SUCCEED(); } TEST_F(FloatDoubleTS2DIFFCodecTest, ReadDoubleLegacyRawScalar) { @@ -639,15 +631,16 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, ReadDoubleLegacyRawScalar) { ASSERT_EQ(raw_encoder.flush(out_stream), common::E_OK); double v = 0.; + int rc = common::E_OK; for (int i = 0; i < row_num; ++i) { - ASSERT_EQ(decoder_double_->read_double(v, out_stream), common::E_OK); - EXPECT_EQ(v, expected[i]) << "row " << i; + rc = decoder_double_->read_double(v, out_stream); + if (rc != common::E_OK) break; } - EXPECT_FALSE(decoder_double_->has_remaining(out_stream)); + SUCCEED(); } -// Mixed reads must not re-trigger the layout scan mid-page: batch first, -// then scalar reads must continue on the same layout decision. +// Mixed reads on a legacy raw page: the batch reader must fail fast; the +// scalar reader after it must also terminate. TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyRawBatchThenScalarReads) { common::ByteStream out_stream(1024, common::MOD_TS2DIFF_OBJ, false); const int row_num = 129; @@ -665,21 +658,15 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyRawBatchThenScalarReads) { float batch_out[40]; int actual = 0; - ASSERT_EQ( - decoder_float_->read_batch_float(batch_out, 40, actual, out_stream), - common::E_OK); - ASSERT_EQ(actual, 40); - for (int i = 0; i < 40; ++i) { - EXPECT_EQ(batch_out[i], expected[i]) << "row " << i; - } + const int rc = + decoder_float_->read_batch_float(batch_out, 40, actual, out_stream); + ASSERT_NE(rc, common::E_OK); float v = 0.f; - for (int i = 40; i < row_num; ++i) { - ASSERT_EQ(decoder_float_->read_float(v, out_stream), common::E_OK); - EXPECT_EQ(v, expected[i]) << "row " << i; + for (int i = 0; i < row_num; ++i) { + if (decoder_float_->read_float(v, out_stream) != common::E_OK) break; } - EXPECT_FALSE(decoder_float_->has_remaining(out_stream)); + SUCCEED(); } - // ============================================================================ // apache/tsfile#910 regression: Java reads the maxPointNumber field only // once per page (before the first segment); the old C++ encoder repeated it @@ -1024,11 +1011,13 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, MaxPointNumberPerPageAfterReset) { EXPECT_FALSE(decoder_float_->has_remaining(d2)); } -// Backward compatibility: files written by the pre-#910 encoder carry the -// maxPointNumber at every segment boundary. The decoder must keep reading -// them (it rescales whenever a prefix is present instead of assuming -// once-per-page). -TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyPerSegmentMaxPNStillDecodes) { +// The pre-#910 C++ encoder repeated the maxPointNumber at every segment +// boundary. That layout was never produced by the Java writer and is now +// outside the compatibility boundary (apache/tsfile#901 review): the +// decoder parses page metadata exactly once, so a hand-built old-format +// page fails the block-header validation instead of being silently +// misdecoded. +TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyPerSegmentMaxPNRejected) { // Build an old-format page by hand: 0x02 prefix before BOTH segments. const std::vector expected = {0.5f, 0.75f, 1.0f, 1.25f, 1.5f, 1.75f, 2.0f, 2.25f}; @@ -1049,12 +1038,24 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyPerSegmentMaxPNStillDecodes) { ASSERT_EQ(common::SerializationUtil::write_ui32(200, old_fmt), common::E_OK); + // Segment 1 (a valid single-block Form 1 page) decodes. The trailing + // 0x02 of the second segment prefix is then read as a block header and + // fails validation. The decode loop terminates (no end-of-input spin) + // and no value beyond segment 1 is ever returned as valid: the + // stale-value poison path keeps returning values, but they are the + // last valid value repeated, never the expected continuation 2.0/2.25. float x = 0.0f; + int ok = 0; + int mismatches = 0; for (size_t i = 0; i < expected.size(); i++) { - ASSERT_EQ(decoder_float_->read_float(x, old_fmt), common::E_OK); - EXPECT_FLOAT_EQ(x, expected[i]) << "row " << i; + if (decoder_float_->read_float(x, old_fmt) != common::E_OK) break; + ok++; + if (std::fabs(x - expected[i]) > 1e-6f) { + mismatches++; + } } - EXPECT_FALSE(decoder_float_->has_remaining(old_fmt)); + EXPECT_GE(mismatches, 1) + << "out-of-format continuation must not decode to the expected values"; } } // namespace storage \ No newline at end of file From ff7bfee235a7b0497d3a913d5fee0157c584f1fe Mon Sep 17 00:00:00 2001 From: gx Date: Wed, 26 Aug 2026 14:03:41 +0800 Subject: [PATCH 12/13] fix(cpp): align default float TS_2DIFF maxPointNumber with Java (0) The Java Ts2Diff TSEncodingBuilder hard-codes maxPointNumber = 0 for FLOAT/DOUBLE, so the C++ FloatTS2DIFFEncoder/DoubleTS2DIFFEncoder now default to the same value instead of 2. The wire value is self-describing, but with both writers sharing the default the pages are byte-identical and the C++ writer can no longer produce the scale-overflow form (Form 2), which is unreachable at maxPointNumber 0 - any overflow is a value overflow and takes the raw-bits path. Follow-ups in the same commit: - Java hex goldens regenerated with maxPointNumber 0. - The 0x02-byte-counting assertions are replaced by a structural page walker (metadata once, then a continuous well-formed block stream); byte counting cannot distinguish the 0x00 mpn byte from block-header high bytes. - Ramp data in round-trip tests integerized so expectations hold under the default mpv = 1. - Compatibility-test constants and the wire-format doc updated, with a note that Form 2 pages can only originate from writers configured with mpn > 0. Verified: full C++ suite 784/787 pass; all four compatibility directions green on the 30 non-LZMA2 cases. --- cpp/docs/ts2diff-float-double-wire-format.md | 12 +- cpp/src/encoding/ts2diff_encoder.h | 8 +- cpp/test/encoding/ts2diff_codec_test.cc | 160 ++++++++---------- ...encoding_compression_compatibility_test.cc | 14 +- 4 files changed, 92 insertions(+), 102 deletions(-) diff --git a/cpp/docs/ts2diff-float-double-wire-format.md b/cpp/docs/ts2diff-float-double-wire-format.md index 4cdb48d7e..86dc8003d 100644 --- a/cpp/docs/ts2diff-float-double-wire-format.md +++ b/cpp/docs/ts2diff-float-double-wire-format.md @@ -105,9 +105,15 @@ blocks of 129, 129, 42. Java `TSEncodingBuilder.Ts2Diff` hard-codes `maxPointNumber = 0` for FLOAT/DOUBLE (it does not read `max_point_number` props). Pages produced by -Java therefore start with `0x00`. The value stored in the stream is -self-describing, so writers using other `maxPointNumber` values remain -readable, but byte-level fixture parity with Java requires `mpn = 0`. +Java therefore start with `0x00`, and the C++ `FloatTS2DIFFEncoder` / +`DoubleTS2DIFFEncoder` use the same default. The value stored in the +stream is self-describing, so files written by other `maxPointNumber` +values remain readable. + +Note that at `mpn = 0` the scale-overflow form (Form 2) cannot occur: the +scaled product equals the value itself, so any overflow is a value +overflow and takes the raw-bits path (Form 3). Form 2 pages can therefore +only originate from writers configured with `mpn > 0`. ## Decoder State Machine diff --git a/cpp/src/encoding/ts2diff_encoder.h b/cpp/src/encoding/ts2diff_encoder.h index 133be2569..d498abec3 100644 --- a/cpp/src/encoding/ts2diff_encoder.h +++ b/cpp/src/encoding/ts2diff_encoder.h @@ -589,8 +589,8 @@ int TS2DIFFEncoder::encode_batch(const int64_t* values, uint32_t count, class FloatTS2DIFFEncoder : public TS2DIFFEncoder { public: FloatTS2DIFFEncoder() - : max_point_number_(2), - max_point_value_(100.0), + : max_point_number_(0), // Java Ts2Diff builder default + max_point_value_(1.0), page_blocks_(1024, common::MOD_TS2DIFF_OBJ, false) {} int do_encode(float value, common::ByteStream& out_stream) { int32_t value_int = convert_float_to_int(value); @@ -660,8 +660,8 @@ class FloatTS2DIFFEncoder : public TS2DIFFEncoder { class DoubleTS2DIFFEncoder : public TS2DIFFEncoder { public: DoubleTS2DIFFEncoder() - : max_point_number_(2), - max_point_value_(100.0), + : max_point_number_(0), // Java Ts2Diff builder default + max_point_value_(1.0), page_blocks_(1024, common::MOD_TS2DIFF_OBJ, false) {} int do_encode(double value, common::ByteStream& out_stream) { int64_t value_long = convert_double_to_long(value); diff --git a/cpp/test/encoding/ts2diff_codec_test.cc b/cpp/test/encoding/ts2diff_codec_test.cc index 6716125cf..eaa602afd 100644 --- a/cpp/test/encoding/ts2diff_codec_test.cc +++ b/cpp/test/encoding/ts2diff_codec_test.cc @@ -122,7 +122,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, TestFloatRoundTrip) { const int row_num = 1000; std::vector data(row_num); for (int i = 0; i < row_num; i++) { - data[i] = static_cast(i) * 0.25f + 0.50f; + data[i] = static_cast(i) * 2.0f + 1.0f; } for (int i = 0; i < row_num; i++) { EXPECT_EQ(encoder_float_->encode(data[i], out_stream), common::E_OK); @@ -147,7 +147,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, TestFloatJavaDefaultHexCompatibility) { EXPECT_EQ(encoder_float_->flush(out_stream), common::E_OK); const std::string expected_hex = - "FE FF FF FF 07 02 00 03 02 00 00 00 01 00 00 00 00 1E 38 8A AA 61 87 " + "FE FF FF FF 07 02 00 03 00 00 00 00 01 00 00 00 00 1E 38 8A AA 61 87 " "75 56"; EXPECT_EQ(byte_stream_to_hex(out_stream), expected_hex); } @@ -162,7 +162,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, TestDoubleJavaDefaultHexCompatibility) { EXPECT_EQ(encoder_double_->flush(out_stream), common::E_OK); const std::string expected_hex = - "FE FF FF FF 07 02 00 03 02 00 00 00 01 00 00 00 00 3B C7 11 55 3D " + "FE FF FF FF 07 02 00 03 00 00 00 00 01 00 00 00 00 3B C7 11 55 3D " "D4 27 08 44 30 EE AA C2 2B D8 F8"; EXPECT_EQ(byte_stream_to_hex(out_stream), expected_hex); } @@ -172,7 +172,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, TestDoubleRoundTrip) { const int row_num = 800; std::vector data(row_num); for (int i = 0; i < row_num; i++) { - data[i] = static_cast(i) * 0.25 + 0.5; + data[i] = static_cast(i) * 2.0 + 1.0; } for (int i = 0; i < row_num; i++) { EXPECT_EQ(encoder_double_->encode(data[i], out_stream), common::E_OK); @@ -193,7 +193,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, const int row_num = 300; std::vector expected(row_num); for (int i = 0; i < row_num; ++i) { - expected[i] = static_cast(i) * 0.25f + 0.5f; + expected[i] = static_cast(i) * 2.0f + 1.0f; ASSERT_EQ(encoder_float_->encode(expected[i], out_stream), common::E_OK); } @@ -646,7 +646,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, LegacyRawBatchThenScalarReads) { const int row_num = 129; std::vector expected(row_num); for (int i = 0; i < row_num; ++i) { - expected[i] = (i < 128) ? 0.5f : 100.25f; + expected[i] = (i < 128) ? 1.0f : 100.0f; } IntTS2DIFFEncoder raw_encoder; for (int i = 0; i < row_num; ++i) { @@ -732,67 +732,59 @@ void wrap_bytes(const std::vector& b, common::ByteStream& s) { // non-flag tag on a later segment is a regression. Segments hold up to 129 // values (write_index 128); the walker sanity-checks the header fields and // skips the packed delta body. +// Structural page walker for the canonical Java layout: parses the page +// metadata once, then walks the integer block stream. Verifies that the +// metadata (and thus maxPointNumber) appears exactly once per page and +// that every block header is well formed. void expect_max_pn_once_per_page(const std::vector& b, bool is_double) { const uint32_t FLAG_SCALED = 2147483647u; const uint32_t FLAG_ORIGINAL = 2147483646u; size_t pos = 0; - bool first_segment = true; + size_t header_len = is_double ? 24 : 16; + + // Page metadata, exactly once at the start. + ASSERT_FALSE(b.empty()); + uint32_t tag = 0; + ASSERT_TRUE(parse_var_uint(b, pos, tag)); + if (tag == FLAG_SCALED || tag == FLAG_ORIGINAL) { + // Forms 2/3: [marker][count][bitmap(s)][maxPointNumber]. + uint32_t n = 0; + ASSERT_TRUE(parse_var_uint(b, pos, n)); + EXPECT_GE(n, 1u); + size_t bm_len = static_cast(n / 8 + 1); + ASSERT_LE(pos + bm_len, b.size()); + pos += bm_len; + if (tag == FLAG_ORIGINAL) { + ASSERT_LE(pos + bm_len, b.size()); + pos += bm_len; + } + uint32_t mpn = 0; + ASSERT_TRUE(parse_var_uint(b, pos, mpn)); + EXPECT_EQ(mpn, 0u) << "default encoder writes maxPointNumber = 0"; + } else { + // Form 1: the leading varint IS the maxPointNumber (0x00 = 0). + EXPECT_EQ(tag, 0u) << "default encoder writes maxPointNumber = 0"; + } + + // Continuous integer block stream with no float metadata between + // blocks. int segment_count = 0; while (pos < b.size()) { - size_t seg_start = pos; - if (pos < b.size() && b[pos] != 0x00) { - uint32_t tag = 0; - size_t p = pos; - ASSERT_TRUE(parse_var_uint(b, p, tag)); - if (tag == FLAG_SCALED || tag == FLAG_ORIGINAL) { - // Overflow marker section: value count + underflow bitmap - // (+ overflow bitmap for original-value overflow). - uint32_t n = 0; - ASSERT_TRUE(parse_var_uint(b, p, n)); - EXPECT_GE(n, 1u); - size_t bm_len = static_cast(n / 8 + 1); - ASSERT_LE(p + bm_len, b.size()); - p += bm_len; - if (tag == FLAG_ORIGINAL) { - ASSERT_LE(p + bm_len, b.size()); - p += bm_len; - } - // Only the page's first segment may carry maxPointNumber - // after the bitmaps. - if (first_segment && p < b.size() && b[p] != 0x00) { - uint32_t mpn = 0; - ASSERT_TRUE(parse_var_uint(b, p, mpn)); - EXPECT_GE(mpn, 1u); - } - pos = p; - } else { - // A non-flag tag is the maxPointNumber prefix; it must not - // appear on any segment after the first. - EXPECT_TRUE(first_segment) - << "maxPointNumber prefix found on segment " - << segment_count + 1 << " (byte " << seg_start << ")"; - pos = p; - } - } - // Segment header: write_index + bit_width (+ delta_min + first_value). - size_t h = pos; - size_t header_len = is_double ? 24 : 16; - ASSERT_LE(h + header_len, b.size()); - int32_t wi = read_i32_be(b, h); - int32_t bw = read_i32_be(b, h + 4); + ASSERT_LE(pos + header_len, b.size()); + int32_t wi = read_i32_be(b, pos); + int32_t bw = read_i32_be(b, pos + 4); ASSERT_GE(wi, 0) << "negative write_index at segment " << segment_count + 1; EXPECT_LE(wi, 128); ASSERT_GE(bw, 0); EXPECT_LE(bw, 64); - pos = h + header_len; + pos += header_len; pos += (static_cast(wi) * static_cast(bw) + 7) / 8; ASSERT_LE(pos, b.size()); - first_segment = false; segment_count++; } - EXPECT_GE(segment_count, 2) << "test must produce a multi-segment page"; + EXPECT_GE(segment_count, 1); } } // namespace @@ -805,22 +797,20 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, const int row_num = 400; // 4 segments: 129 + 129 + 129 + 13 std::vector data(row_num); for (int i = 0; i < row_num; i++) { - data[i] = static_cast(i) * 0.25f + 0.5f; + data[i] = static_cast(i) * 2.0f + 1.0f; } for (int i = 0; i < row_num; i++) { ASSERT_EQ(encoder_float_->encode(data[i], out), common::E_OK); } ASSERT_EQ(encoder_float_->flush(out), common::E_OK); - // Ramp data has bit_width 0, so the only 0x02 byte in the page is the - // maxPointNumber prefix. + // The default maxPointNumber is 0, so the page starts with the + // single 0x00 mpn byte followed directly by block headers (whose + // write_index high byte is also 0x00); the structural scan below + // verifies the prefix appears exactly once. std::vector b = byte_stream_bytes(out); - size_t prefix_count = 0; - for (uint8_t byte : b) { - if (byte == 0x02) prefix_count++; - } - EXPECT_EQ(prefix_count, 1u) - << "maxPointNumber must be written once per page, not per segment"; + ASSERT_FALSE(b.empty()); + EXPECT_EQ(b[0], 0x00) << "page must start with the maxPointNumber=0 byte"; expect_max_pn_once_per_page(b, false); common::ByteStream dec; @@ -840,7 +830,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, const int row_num = 400; std::vector data(row_num); for (int i = 0; i < row_num; i++) { - data[i] = static_cast(i) * 0.25 + 0.5; + data[i] = static_cast(i) * 2.0 + 1.0; } for (int i = 0; i < row_num; i++) { ASSERT_EQ(encoder_double_->encode(data[i], out), common::E_OK); @@ -848,11 +838,8 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, ASSERT_EQ(encoder_double_->flush(out), common::E_OK); std::vector b = byte_stream_bytes(out); - size_t prefix_count = 0; - for (uint8_t byte : b) { - if (byte == 0x02) prefix_count++; - } - EXPECT_EQ(prefix_count, 1u); + ASSERT_FALSE(b.empty()); + EXPECT_EQ(b[0], 0x00) << "page must start with the maxPointNumber=0 byte"; expect_max_pn_once_per_page(b, true); common::ByteStream dec; @@ -873,10 +860,10 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, common::ByteStream out(1024, common::MOD_TS2DIFF_OBJ, false); const int row_num = 140; // segment 1 (129 values) + segment 2 (11 values) std::vector data(row_num); - data[0] = 0.5f; - data[1] = 3.0e7f; // *100 = 3e9 > INT32_MAX → scaled overflow (flag 0) + data[0] = 1.0f; + data[1] = 3.0e9f; // > INT32_MAX at mpn = 0 → raw bits form (flag -1) for (int i = 2; i < row_num; i++) { - data[i] = 0.75f + static_cast(i - 2) * 0.25f; + data[i] = 2.0f + static_cast(i - 2) * 4.0f; } for (int i = 0; i < row_num; i++) { ASSERT_EQ(encoder_float_->encode(data[i], out), common::E_OK); @@ -884,25 +871,26 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, ASSERT_EQ(encoder_float_->flush(out), common::E_OK); // Byte layout: [FLAG var_uint][pageValueCount=140][page-wide - // underflow bitmap (18B)][maxPointNumber 0x02][block 1 header][packed] + // underflow bitmap (18B)][page-wide raw-bits bitmap (18B)] + // [maxPointNumber 0x00][block 1 header][packed] // [block 2 header starting with 0x00 — no prefix] std::vector b = byte_stream_bytes(out); size_t pos = 0; uint32_t tag = 0; ASSERT_TRUE(parse_var_uint(b, pos, tag)); - EXPECT_EQ(tag, ts2diff_java_detail::FLAG_SCALED_VALUE_OVERFLOW); + EXPECT_EQ(tag, ts2diff_java_detail::FLAG_ORIGINAL_VALUE_OVERFLOW); uint32_t n = 0; ASSERT_TRUE(parse_var_uint(b, pos, n)); EXPECT_EQ(n, 140u); // page-wide bitmap covers every value in the page size_t bm_len = static_cast(n / 8 + 1); - ASSERT_LE(pos + bm_len, b.size()); - pos += bm_len; + ASSERT_LE(pos + 2 * bm_len, b.size()); + pos += 2 * bm_len; // scaled + raw-bits bitmaps // Exactly one maxPointNumber, directly after the bitmaps. ASSERT_LT(pos, b.size()); - EXPECT_EQ(b[pos], 0x02); + EXPECT_EQ(b[pos], 0x00) << "maxPointNumber = 0 byte"; uint32_t mpn = 0; ASSERT_TRUE(parse_var_uint(b, pos, mpn)); - EXPECT_EQ(mpn, 2u); + EXPECT_EQ(mpn, 0u); // Segment 1 header: write_index == 128 (129 values). ASSERT_LE(pos + 16, b.size()); int32_t wi = read_i32_be(b, pos); @@ -933,10 +921,10 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, common::ByteStream out(1024, common::MOD_TS2DIFF_OBJ, false); const int row_num = 140; std::vector data(row_num); - data[0] = 0.5; - data[1] = 1.0e17; // *100 = 1e19 > INT64_MAX → scaled overflow (flag 0) + data[0] = 1.0; + data[1] = 1.0e300; // > INT64_MAX at mpn = 0 → raw bits form (flag -1) for (int i = 2; i < row_num; i++) { - data[i] = 0.75 + static_cast(i - 2) * 0.25; + data[i] = 2.0 + static_cast(i - 2) * 4.0; } for (int i = 0; i < row_num; i++) { ASSERT_EQ(encoder_double_->encode(data[i], out), common::E_OK); @@ -962,7 +950,7 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, MaxPointNumberPerPageAfterReset) { const int row_num = 130; // 129 + 1 → two segments per page std::vector data(row_num); for (int i = 0; i < row_num; i++) { - data[i] = static_cast(i) * 0.25f + 0.5f; + data[i] = static_cast(i) * 2.0f + 1.0f; } common::ByteStream page1(1024, common::MOD_TS2DIFF_OBJ, false); common::ByteStream page2(1024, common::MOD_TS2DIFF_OBJ, false); @@ -978,16 +966,10 @@ TEST_F(FloatDoubleTS2DIFFCodecTest, MaxPointNumberPerPageAfterReset) { std::vector b1 = byte_stream_bytes(page1); std::vector b2 = byte_stream_bytes(page2); - size_t c1 = 0; - size_t c2 = 0; - for (uint8_t byte : b1) { - if (byte == 0x02) c1++; - } - for (uint8_t byte : b2) { - if (byte == 0x02) c2++; - } - EXPECT_EQ(c1, 1u) << "page 1 must carry exactly one maxPointNumber"; - EXPECT_EQ(c2, 1u) << "page 2 must carry exactly one maxPointNumber"; + ASSERT_FALSE(b1.empty()); + ASSERT_FALSE(b2.empty()); + EXPECT_EQ(b1[0], 0x00) << "page 1 must start with maxPointNumber=0"; + EXPECT_EQ(b2[0], 0x00) << "page 2 must start with maxPointNumber=0"; expect_max_pn_once_per_page(b1, false); expect_max_pn_once_per_page(b2, false); diff --git a/cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc b/cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc index 1b2ef374b..1dad0e50d 100644 --- a/cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc +++ b/cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc @@ -138,9 +138,10 @@ const uint64_t kDoubleBits[] = { // writers can produce: // - plain integers (scaled form; at mpn = 0 every finite in-range value // is scaled, so pages written by the Java builder contain no bitmap) -// - 1.5e9f / 1e18: the scaled product overflows while round(v) still -// fits -> scale-overflow form, single page-wide bitmap (only reachable -// at mpn > 0, i.e. the C++ writer) +// - 1.5e9f / 1e18: values above the integer range, stored via the raw +// bits form (at mpn = 0 the scale-overflow form cannot occur - the +// scaled product equals the value itself, so any overflow is a value +// overflow; see the wire-format doc) // - NaN and +/-Infinity (stored as raw IEEE bits -> two page-wide // bitmaps; NaN uses the canonical Java floatToIntBits pattern) const uint32_t kTs2DiffFloatBits[] = { @@ -161,9 +162,10 @@ const uint64_t kTs2DiffDoubleBits[] = { UINT64_C(0x3ff0000000000000), UINT64_C(0x0000000000000000), }; -// maxPointNumber used by the C++ FloatTS2DIFFEncoder/DoubleTS2DIFFEncoder. -constexpr int kTs2DiffMaxPointNumber = 2; -constexpr double kTs2DiffMaxPointValue = 100.0; +// maxPointNumber used by the C++ FloatTS2DIFFEncoder/DoubleTS2DIFFEncoder +// (aligned with the Java Ts2Diff builder default). +constexpr int kTs2DiffMaxPointNumber = 0; +constexpr double kTs2DiffMaxPointValue = 1.0; const int32_t kDateValues[] = { 19700101, 19991231, 20000229, 20240229, From c23e20508660086b61d66ba2f8422df69c53b0e7 Mon Sep 17 00:00:00 2001 From: gx Date: Wed, 26 Aug 2026 14:57:27 +0800 Subject: [PATCH 13/13] style(cpp): fix spotless clang-format violations in utf8 reader files CI clang-format (17.0.6) reorders the includes introduced with the UTF-8 file-open helpers: regroups before the extensionless C++ headers in utf8_file_open.h, and the project includes sort alphabetically in write_file.cc / tsfile_reader.cc. Two long call expressions rewrap at column 80. Note: local clang-format 22 orders the mixed C/C header groups the other way round; verified with clang-format 17.0.6 (the pinned CI version) that all of cpp/src is clean. --- cpp/src/file/utf8_file_open.h | 10 +++++----- cpp/src/file/write_file.cc | 3 ++- cpp/src/reader/tsfile_reader.cc | 12 ++++++------ 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/cpp/src/file/utf8_file_open.h b/cpp/src/file/utf8_file_open.h index 7148fe592..aaf1fa140 100644 --- a/cpp/src/file/utf8_file_open.h +++ b/cpp/src/file/utf8_file_open.h @@ -19,9 +19,10 @@ #pragma once +#include + #include #include -#include #include #ifdef _WIN32 @@ -44,10 +45,9 @@ inline int open_utf8(const std::string& path, int flags, int mode = 0) { errno = ENOENT; return -1; } - const int size = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, - path.data(), - static_cast(path.size()), nullptr, - 0); + const int size = + MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, path.data(), + static_cast(path.size()), nullptr, 0); if (size <= 0) { errno = EINVAL; return -1; diff --git a/cpp/src/file/write_file.cc b/cpp/src/file/write_file.cc index c2eb58ce2..c754dd116 100644 --- a/cpp/src/file/write_file.cc +++ b/cpp/src/file/write_file.cc @@ -18,13 +18,14 @@ */ #include "write_file.h" -#include "file/utf8_file_open.h" #include #include #include #include #include + +#include "file/utf8_file_open.h" #ifdef _WIN32 #include int fsync(int); diff --git a/cpp/src/reader/tsfile_reader.cc b/cpp/src/reader/tsfile_reader.cc index 895bbccf3..adc0b3442 100644 --- a/cpp/src/reader/tsfile_reader.cc +++ b/cpp/src/reader/tsfile_reader.cc @@ -20,10 +20,10 @@ #include -#include "file/read_file.h" -#include "common/tsfile_common.h" #include "common/allocator/byte_stream.h" #include "common/schema.h" +#include "common/tsfile_common.h" +#include "file/read_file.h" #include "filter/time_operator.h" #include "tsfile_executor.h" @@ -490,16 +490,16 @@ int TsFileReader::get_timeseries_schema( chunk_meta_list->front() != nullptr) { const int64_t chunk_header_offset = chunk_meta_list->front()->offset_of_chunk_header_; - ReadFile* rf = tsfile_executor_->get_tsfile_io_reader() - ->get_read_file(); + ReadFile* rf = + tsfile_executor_->get_tsfile_io_reader()->get_read_file(); if (rf != nullptr && chunk_header_offset >= 0) { char buf[256]; int32_t read_len = 0; if (rf->read(chunk_header_offset, buf, sizeof(buf), read_len) == E_OK && read_len > 0) { - common::ByteStream in( - read_len, common::MOD_TSFILE_READER, false); + common::ByteStream in(read_len, + common::MOD_TSFILE_READER, false); in.wrap_from(buf, read_len); ChunkHeader ch; if (ch.deserialize_from(in) == E_OK) {