Skip to content

fix(cpp): Float/DoubleTS2DIFFEncoder writes maxPointNumber per segment, breaking Java FloatDecoder on multi-segment pages #910

Description

@kkzi

[cpp] Float/DoubleTS2DIFFEncoder writes maxPointNumber on every 128-value segment; Java FloatDecoder reads it once per page -> multi-segment DOUBLE/FLOAT pages crash Java readers (selfCheck / TsFileSketchTool)

Summary

The C++ FloatTS2DIFFEncoder::flush / DoubleTS2DIFFEncoder::flush unconditionally write the
maxPointNumber varint at the start of every 128-value segment, while the Java encoding
contract (FloatEncoder.saveMaxPointNumber / FloatDecoder.readMaxPointValue) writes/reads it
exactly once per page. As a result, any FLOAT/DOUBLE column with more than 128 values per page
(more than one TS_2DIFF segment) written by the C++ encoder is unreadable by Java readers:
TsFileSequenceReader.selfCheck fails and TsFileSketchTool reports the file as crashed.

This is a data-format incompatibility of the C++ encoder. The C++ decoder already contains
a workaround for the reverse direction (scan_java_float_double_page in ts2diff_decoder.h,
added in commit 1ef5e94), whose comment explicitly acknowledges the formats differ, but the
encoder was never made Java-compatible.

Java side (the contract)

  • FloatEncoder.encode(double) calls saveMaxPointNumber(out) for every value, but the flag
    guard means the varint is written only once per page:
    // FloatEncoder.java:211
    private void saveMaxPointNumber(ByteArrayOutputStream out) {
      if (!isMaxPointNumberSaved) {
        ReadWriteForEncodingUtils.writeUnsignedVarInt(maxPointNumber, out);
        isMaxPointNumberSaved = true;
      }
    }
    isMaxPointNumberSaved is reset only in flush() (FloatEncoder.java:198) -> once per page.
  • FloatDecoder.readDouble reads it once per page with the symmetric guard:
    // FloatDecoder.java:132
    private void readMaxPointValue(ByteBuffer buffer) {
      if (!isMaxPointNumberRead) {
        int maxPointNumber = ReadWriteForEncodingUtils.readUnsignedVarInt(buffer);
        ...
        isMaxPointNumberRead = true;
      }
    }
  • The page data after the prefix is decoded by DeltaBinaryDecoder.LongDeltaDecoder.loadIntBatch
    (DeltaBinaryDecoder.java:198) as repeated batches of
    [packNum i32][packWidth i32][minDeltaBase i64][firstValue i64] + bit data, big-endian.

C++ side (the bug)

// cpp/src/encoding/ts2diff_encoder.h:780 (Float) and :867 (Double)
FORCE_INLINE int DoubleTS2DIFFEncoder::flush(common::ByteStream& out_stream) {
    ...
    if (RET_FAIL(common::SerializationUtil::write_var_uint(
            static_cast<uint32_t>(max_point_number_), inner))) {   // <- line 875, on EVERY flush
        return ret;
    }
    ...
    if (RET_FAIL(common::SerializationUtil::write_i32(write_index_, inner))) ...

flush() is invoked once per 128-value block, so every segment carries its own
maxPointNumber prefix. Java reads the prefix only once; the second segment's 0x02 prefix byte is
then mis-parsed as the leading byte of the next batch packNum field.

Byte-level evidence

Value column of a real chunk (marker=0x05, DOUBLE, TS_2DIFF, 204 points, uncompressed), 50 bytes:

02 | 00 00 00 80 | 00 00 00 00 | 00 00 00 00 00 00 00 00 | 00 00 00 00 00 00 00 05   <- segment 1 (128 values)
02 | 00 00 00 4c | 00 00 00 00 | 00 00 00 00 00 00 00 00 | 00 00 00 00 00 00 00 05   <- segment 2 (76 values)

What Java's FloatDecoder actually reads:

batch0: packNum=128   packWidth=0    minDeltaBase=0 firstValue=5    -> 128 values of 0.05, OK
batch1: packNum=33554432 packWidth=1275068416  minDeltaBase=0 firstValue=5   <- mis-parse!
        (the second 0x02 prefix was consumed as the high byte of packNum)

encodingLength = ceil(packNum*packWidth) overflows to 0, deltaBuf is empty, and the very first
bit read in readPack -> BytesUtils.bytesToLong(deltaBuf, 0, 1275068416) accesses deltaBuf[0]
and throws ArrayIndexOutOfBoundsException (message "0" on JDK 8).

TsFileSequenceReader.selfCheck (single-page branch, TsFileSequenceReader.java:2519) decodes the
page via PageReader.getAllSatisfiedPageData, catches the exception, logs:

WARN  ...TsFileSequenceReader - TsFile ... self-check cannot proceed at position 203764 recovered, because : 0

and returns a non-COMPLETE status, so TsFileSketchTool (via error.utils.sketch_tool_cannot_load)
reports:

java.io.IOException: Cannot load file ... because the file has crashed.

Minimal reproduction

Program (uses the public TsFileWriter + Tablet API, no internal APIs):

#include "writer/tsfile_writer.h"
#include "common/schema.h"
#include "common/tablet.h"
#include "file/write_file.h"
#include "common/tsfile_common.h"
// usage: repro <num_rows> <out.tsfile>
// ... open writer, register DOUBLE/TS_2DIFF/UNCOMPRESSED timeseries "s" under "root.repro",
//     write <num_rows> rows via Tablet, flush, close.

Build against the repo's C++ library and run:

$ repro_ts2diff 200 ts2diff_200.tsfile     # 200 values -> page with 2 segments (128 + 72)
write_tablet rc=0
flush rc=0
close rc=0
done. rows=200 file=ts2diff_200.tsfile

Verify with the Java tooling (tsfile-all jar):

$ java -cp 'tsfile-all-*.jar' org.apache.tsfile.utils.TsFileSketchTool ts2diff_200.tsfile
WARN  o.a.t.read.TsFileSequenceReader - TsFile ... ts2diff_200.tsfile self-check cannot proceed at position 128 recovered, because : 0
java.io.IOException: Cannot load file ...ts2diff_200.tsfile because the file has crashed.

With 100 values (single segment) the same program produces a file that passes self-check; with
200/204 values it fails exactly as above.

Why this has been hidden

Whether Java crashes or silently mis-decodes depends on the sign of the mis-parsed packWidth
(= second segment's write_index byte shifted left 24):

  • second segment write_index < 128 -> packWidth positive -> bytesToLong loops on the empty
    deltaBuf -> crash (this is the common case for page sizes between 129 and 255 points).
  • second segment write_index >= 128 -> packWidth negative -> the while (width > 0) loop in
    BytesUtils.bytesToLong is skipped, no exception, and Java silently returns wrong values.

The existing C++ tests are self-consistent (C++ writes, C++ reads) so they never detect it, and
the C++ decoder-side scan_java_float_double_page handles Java-written files only.

Suggested fix

Make the C++ encoder write maxPointNumber once per page (e.g., guard it with a
first-flush flag, mirroring Java's isMaxPointNumberSaved), so the emitted
FLOAT/DOUBLE TS_2DIFF stream matches what Java FloatDecoder expects. The existing decoder-side
scan_java_float_double_page logic then remains correct for reading Java-written files.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions