fix(cpp): TS_2DIFF float/double maxPointNumber once per page (fixes #910)

[kkzi]

Fix C++ TS_2DIFF float/double encoding to match the Java layout, and make the decoder tolerate all three page layouts. Fixes #910.

Summary

• Encoder: write the maxPointNumber var_uint (fixed value 2) exactly once per page instead of at every segment boundary, matching Java FloatEncoder/DoubleEncoder. The Java readers (e.g. TsFileSketchTool) crash on the old layout when a page's first segment is empty/short — that is fix(cpp): Float/DoubleTS2DIFFEncoder writes maxPointNumber per segment, breaking Java FloatDecoder on multi-segment pages #910. The encoder writes maxPN at page start (first encode after reset), so every non-empty page begins with either a FLAG section or the maxPN prefix — this is the invariant the decoder relies on.
• Decoder: forward-only prefix-aware parsing that accepts legacy raw pages (no prefix, first byte 0x00), the new Java layout (maxPointNumber only on the page's first segment), and the old C++ per-segment format (backward compatible). A prefix-free segment's header is preloaded so decode() never rewinds the stream.
  • Note on approach: this replaces the earlier whole-page scan (scan_java_float_double_page in 1ef5e94). The scan assumed every segment carries a prefix and could not bound segment boundaries on new-format pages (segments 2+ have no prefix and no separator), which made scaled-overflow pages (FLAG + prefix-free continuation) unreadable. The forward-only parser dispatches on the first byte (0x00 / 0x02 / FLAG) with a per-page page_first_segment_ flag, which handles the new layout unambiguously.
• ByteStream: check_space() recomputes the read page from the head instead of blindly following next_ when the cursor is parked at a page boundary (from 1ef5e94). The decoder's probe rewinds (set_read_pos fallback branches) rely on this.
• Tests: new gtest cases assert the once-per-page byte layout for multi-segment pages, scaled-overflow pages (the fix(cpp): Float/DoubleTS2DIFFEncoder writes maxPointNumber per segment, breaking Java FloatDecoder on multi-segment pages #910 crash scenario), reset() page boundaries, and legacy per-segment backward compatibility. Legacy raw batch/scalar/mixed regressions from the earlier review are kept.

Verification

• Full C++ test suite: 763/763 pass.
• Java TsFileSketchTool reads files written by the fixed encoder (previously crashed).
• tsfile_cli round-trips the data.

[ColinLeeo]

Thanks for tracking this down.

The root-cause analysis is clear, and the new implementation correctly handles Java-compatible prefixes, including overflow prefixes and reads spanning multiple segments.

I found one blocking compatibility issue, though: routing FLOAT/DOUBLE batch reads through the scalar decoder regresses legacy raw segments. The scalar prefix detector can misclassify a valid raw header, after which the decoder gets an invalid bit_width_ and spins at end-of-input.

I reproduced this for both FLOAT and DOUBLE by encoding 129 sequential raw bit patterns with IntTS2DIFFEncoder / LongTS2DIFFEncoder, then reading them in small batches through the corresponding floating-point decoder. The PR head hangs, while the parent implementation completes successfully.

Could we preserve the integer batch path for legacy raw segments, or make the prefix detection unambiguous before switching to the scalar path? It would also be good to add legacy raw batch regression tests for both types.

[ColinLeeo]

The overall fix direction looks good, but the legacy raw segment compatibility issue is not fully addressed yet.

[kkzi]

Hi @ColinLeeo, thanks for the thorough review and the reproduction steps — they made this straightforward to chase down. I've pushed 1ef5e94 addressing all three points.

Root cause confirmed. Your repro hangs exactly as described: the per-block heuristic (looks_like_ts2diff_header) only validated a single misaligned block header (wi/bw range check). A legacy raw block with wi = 0 (or bit_width = 0, i.e. any constant-value block) passes that probe with all-zero bytes, gets misread as a maxPointNumber prefix, and the desync cascades into the end-of-input spin.

Fix — unambiguous prefix detection (your option 2). The layout is now decided once per page by scan_java_float_double_page(), which parses the entire remaining stream with the Java segment grammar: [overflow flag][count][bitmaps][mpn] block+, with field-range validation, Σ(wi+1) == bitmap count for overflow segments, and exact whole-stream exhaustion. A page only counts as Java-compatible when the grammar consumes it exactly. A real legacy raw page fails this immediately: after the varint tag eats the leading 0x00, the misaligned write_index probe reads >= 0x100 and is rejected. The detected prefix offsets are recorded and the decoder only consumes prefixes at those offsets, which also fixes value semantics for Java single-segment multi-block pages the per-block heuristic couldn't represent.

Legacy raw batch path preserved (your option 1). Legacy raw pages route through the integer SIMD batch decoder + bit-cast, exactly the parent-commit behavior; Java pages take the segment-aware scalar path.

Regression tests. Added for both FLOAT and DOUBLE:

• ReadBatchFloatLegacyRawSegments / ReadBatchDoubleLegacyRawSegments — 129 values via IntTS2DIFFEncoder/LongTS2DIFFEncoder (128-value constant block + trailing change, hitting both the bit_width = 0 and wi = 0 misclassification patterns), read in small batches of 16
• ReadFloatLegacyRawScalar / ReadDoubleLegacyRawScalar — scalar path across the block boundary
• LegacyRawBatchThenScalarReads — mixed batch/scalar reads on one page

Also hardened read_long() to bail out when the stream is exhausted with bits still owed, so no residual misconfiguration can loop forever.

One incidental fix this surfaced: ByteStream::check_space() skipped a page when set_read_pos() parked the cursor at a page boundary (it blindly followed read_page_->next_, yielding E_OUT_OF_RANGE on the next read). The scan-based detection depends on position restore, so I fixed it to recompute the page from the head.

Full C++ suite (757 tests) passes, and clang-format --dry-run --Werror is clean. Happy to adjust if you'd prefer a different split.

[kkzi]

Thanks for the review. I have pushed a66a679 which fixes the spotless clang-format violations flagged by CI (ts2diff_decoder.h and ts2diff_codec_test.cc).

The CI runs for this new commit are currently waiting for approval (action_required) — could you approve the workflows so they can re-run?

Happy to address any remaining feedback on the legacy-raw compatibility path.

[ColinLeeo]

One more thought regarding the compatibility design: do we really need to support the historical C++ TS_2DIFF layout?
Since Java implementation is the reference for TsFile format compatibility, I think the previous C++ behavior (writing maxPointNumber for every block) should be considered as a C++ implementation bug rather than a legacy format that needs to be preserved.
Supporting this old layout introduces additional format detection logic and ambiguity in the decoder. It may make the implementation harder to maintain.
Would it be possible to simplify this PR by:

• making C++ writer follow the Java layout exactly;
• making C++ reader support the Java layout only;
• removing the old C++ layout compatibility path?

Then the regression tests can focus on Java ↔ C++ interoperability.

[ColinLeeo]

Hi, @kkzi

Clarification of the FLOAT/DOUBLE TS_2DIFF Format and the Direction of This Fix

TL;DR

• The Java FLOAT/DOUBLE + TS_2DIFF layout is the canonical layout for cross-language compatibility. The C++ writer and reader use this format as their compatibility boundary.
• The compatibility scope does not include the raw bit-cast layout produced by the earlier C++ FLOAT/DOUBLE + TS_2DIFF writer.

# C++ layout introduced by #796
[overflow marker 1][blockValueCount 1][bitmap(s) 1][maxPointNumber][block 1]
[overflow marker 2][blockValueCount 2][bitmap(s) 2][maxPointNumber][block 2]
[overflow marker 3][blockValueCount 3][bitmap(s) 3][maxPointNumber][block 3]

# C++ layout in the current PR
[overflow marker 1][blockValueCount 1][bitmap(s) 1][maxPointNumber][block 1]
[overflow marker 2][blockValueCount 2][bitmap(s) 2]                [block 2]
[overflow marker 3][blockValueCount 3][bitmap(s) 3]                [block 3]

# Canonical Java layout
[overflow marker][pageValueCount][page-wide bitmap(s)]
[maxPointNumber][block 1][block 2][block 3]

I reviewed the Java and C++ encoder/decoder implementations and their history again. In the earlier discussion, I mixed together the official Java FLOAT/DOUBLE format, the raw format produced by the early C++ writer, and the per-block prefix format produced by the later C++ writer. I apologize for the confusion. The sections below describe these formats separately and note the remaining boundaries in the current code.

1. How Java Handles FLOAT/DOUBLE TS_2DIFF

TS_2DIFF itself encodes integers. Java adds a FLOAT/DOUBLE wrapper around it:

FLOAT  -> int32 -> IntDeltaEncoder
DOUBLE -> int64 -> LongDeltaEncoder

Suppose maxPointNumber = 2, so maxPointValue = 100. Java handles each value in one of three ways:

Condition	Stored representation	Decoding
value * 100 fits in the target integer type	round(value * 100)	Divide by 100
The scaled value overflows, but the original value fits in the target integer type	round(value)	Divide by 1
The original value cannot be converted safely, or it is NaN/Infinity	Result of floatToIntBits / doubleToLongBits	Restore from the raw bits

To distinguish these cases, Java writes one or two page-wide bitmaps when needed.

One detail is that Java uses Float.floatToIntBits / Double.doubleToLongBits. Regular values and Infinity retain their corresponding IEEE 754 bit patterns, while NaN is converted to Java's canonical NaN bit pattern.

The Java page layout has three forms:

# Every value can be scaled normally
[maxPointNumber]
[TS_2DIFF block 1][TS_2DIFF block 2]...

# At least one scaled value overflows, but no original value overflows
[Integer.MAX_VALUE]
[pageValueCount]
[page-wide scaled-value bitmap]
[maxPointNumber]
[TS_2DIFF block 1][TS_2DIFF block 2]...

# At least one value is stored as its raw IEEE 754 bits
[Integer.MAX_VALUE - 1]
[pageValueCount]
[page-wide scaled-value bitmap]
[page-wide original-value bitmap]
[maxPointNumber]
[TS_2DIFF block 1][TS_2DIFF block 2]...

The key points are:

• maxPointNumber appears once per page.
• The overflow flags and bitmaps are also page-level metadata.
• Each bitmap covers the entire page rather than one TS_2DIFF block.
• The metadata is followed by a continuous sequence of integer TS_2DIFF blocks, with no additional FLOAT/DOUBLE prefix between blocks.

2. The Previously Mentioned Raw TS_2DIFF Path

Before #796, the C++ FloatTS2DIFFEncoder / DoubleTS2DIFFEncoder interpreted each floating-point value as an integer of the same width and then applied integer TS_2DIFF:

float IEEE bits -> int32 -> integer TS_2DIFF -> int32 -> float IEEE bits

double IEEE bits -> int64 -> integer TS_2DIFF -> int64 -> double IEEE bits

This approach preserves the floating-point bits losslessly. It is not the Java FLOAT/DOUBLE TS_2DIFF format, but it was once the official C++ writer output when FLOAT/DOUBLE + TS_2DIFF was selected explicitly.

My earlier regression test used IntTS2DIFFEncoder / LongTS2DIFFEncoder to encode the same IEEE bit patterns, producing a payload equivalent to this earlier writer path. The test demonstrated that the new prefix detection could misclassify a historical raw block as a Java floating-point prefix and eventually cause the decoder to hang.

The current writer no longer produces the raw layout, while the reader's handling of it represents compatibility logic for historical C++ files.

The raw layout was private to the early C++ implementation and was never supported by the Java reader, so it is not part of the cross-language TsFile format. Detecting the raw and Java layouts from the input bytes retains this historical behavior in the decoder state machine and also introduces format ambiguity.

From the perspective of format boundaries and implementation complexity, I prefer focusing the C++ decoder on the canonical Java layout and returning a format error for the earlier raw layout. This simplifies prefix handling and avoids continuing to decode after a misclassification. If the community wants to retain support for early C++ files, that behavior can be discussed separately with a more explicit format identifier.

3. C++ Writer Layout After the Java-Style Wrapper Was Introduced

#796 changed C++ FLOAT/DOUBLE TS_2DIFF from raw bit-casting to Java-style scaling, maxPointNumber, and overflow bitmaps, followed by integer TS_2DIFF.

However, the integer encoder triggers a block flush after accumulating 129 values, and FloatTS2DIFFEncoder::flush() also writes the floating-point wrapper metadata.

As a result, this version of the C++ writer produces a per-block layout. For a page containing three blocks where every block has an overflow value, the layout is:

[overflow marker 1][blockValueCount 1][bitmap(s) 1][maxPointNumber][block 1]
[overflow marker 2][blockValueCount 2][bitmap(s) 2][maxPointNumber][block 2]
[overflow marker 3][blockValueCount 3][bitmap(s) 3][maxPointNumber][block 3]

A block without overflow uses:

[maxPointNumber][block]

This layout results from the integer block flush and the FLOAT/DOUBLE wrapper flush sharing the same flush() method. After every 129 values, the integer encoder invokes the virtual flush() method. During that call, the floating-point encoder generates the bitmap for the current block, writes it, and clears underflow_flags_. The next block collects a new set of flags and generates another bitmap.

The difference from the Java page-wide layout is the metadata scope. This discussion uses the Java layout as the format baseline: the target C++ encoder/decoder layout has page-wide metadata, while the earlier C++ per-block layout is outside the compatibility scope.

The default encoding for FLOAT/DOUBLE is GORILLA. An explicit TS_2DIFF schema choice still selects this C++ writer path.

4. Format Differences That Remain in the Current PR

This PR uses max_point_number_saved_ to avoid writing maxPointNumber repeatedly for every block. A multi-block page without overflow now has this layout:

[maxPointNumber][block 1][block 2]...

This part matches Java.

One remaining difference concerns metadata scope: underflow_flags_ is still cleared after each block flush, and each overflow bitmap is still generated per block. When overflow occurs, C++ therefore continues to write per-block metadata rather than Java's page-wide metadata.

The expanded comparison of the three layouts appears in the TL;DR.

The current PR removes the repeated maxPointNumber before later blocks, but the bitmaps remain per-block, so the resulting layout still differs from Java.

The decoder follows the same per-block model. When entering a later block, it clears the bitmap and resets segment_pos_ to 0. When reading a multi-block overflow page produced by Java, it loses the page-wide bitmap state after the first block.

Another related boundary is maxPointNumber = 0. The Java encoder supports this configuration, for which the first byte of a canonical page is 0x00. The current prefix detection treats a leading 0x00 byte as a legacy raw payload. For a simple payload whose first value is 1.0, the C++ decoder returns E_OK but produces approximately 2.35099e-38.

For canonical Java format compatibility, the current PR has completed the change that writes maxPointNumber once per page. The overflow metadata and decoder bitmap state remain block-scoped.

5. One Possible Implementation Direction

The encoder could collect floating-point conversion state across the entire page and generate the flags, bitmaps, and maxPointNumber once during the page flush, followed by a continuous sequence of integer TS_2DIFF blocks.

The decoder could parse the FLOAT/DOUBLE metadata once at the beginning of the page and retain the bitmap and current value position throughout the page. When it enters a new integer block, it would continue using the same page-wide bitmap and position.

The C++ implementation uses the canonical Java layout as its format baseline. The automatic detection and fallback logic that distinguishes the raw layout, the earlier C++ per-block layout, and the Java layout can be simplified at the same time. The decoder then maintains only the Java format state machine, while other inputs return a format error.

The existing batch decoder remains reusable:

1. Parse the page-level FLOAT/DOUBLE metadata once
2. Decode the integer TS_2DIFF blocks with read_batch_int32/read_batch_int64
3. Look up the bitmap using the page-wide position
4. Convert the integers in the batch to FLOAT/DOUBLE

Per-value bitmap checks and numeric conversion remain, while the main bit unpacking, delta reconstruction, and SIMD batch paths can still be reused. FLOAT/DOUBLE batch reads can also reuse the main flow of the integer batch decoder.

6. Additional Validation for the Current PR

The existing Java/C++ compatibility test can be reused. Relevant locations include:

• Java: java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java
• C++: cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc
• Workflow: .github/workflows/compatibility-test.yml

At present, Java's buildMatrix() and C++'s BuildMatrix() cover only CHIMP, RLBE, and CAMEL. They do not include FLOAT/DOUBLE + TS_2DIFF. Both sides also write only 32 points by default, so the tests do not cross the boundary of a TS_2DIFF block containing 129 values.

Two additions can extend this coverage:

encoding matrix += FLOAT + TS_2DIFF
encoding matrix += DOUBLE + TS_2DIFF
rowCount = 300  # apply to all compatibility cases

In other words, the matrices on both sides can include FLOAT/DOUBLE + TS_2DIFF, while the number of written points for every existing compatibility case can be increased to 300. These cases can continue using the existing fixture generation, manifest, and bidirectional validation flow.

One detail is worth noting: the current compatibility test uses exact-bit comparisons for FLOAT/DOUBLE, while TS_2DIFF applies a fixed-point conversion based on maxPointNumber. Values such as -0.0 and pi in the existing data may not retain their original bits after TS_2DIFF encoding. The new combinations can either use data that is restored exactly after scaling or reflect the TS_2DIFF conversion rules in their expected values.

To cover the page-wide bitmap across blocks, the compatibility cases can use rowCount = 300 and include scaled-overflow data among those 300 values. This crosses the 129-value block boundary and exercises both Java page-wide bitmap reads and C++ multi-block writer output.

A Java fixture with maxPointNumber = 0 can also cover the prefix boundary. It verifies that a canonical page beginning with 0x00 enters the correct parsing path.

I plan to cover a more complete cross-language compatibility matrix in a separate follow-up PR, including parameterized row counts, data types, encodings, and compression combinations. If increasing the row count reveals compatibility issues in other combinations, those can be tracked in separate issues.