Skip to content

Restrict compatibility matrix to the table model and finish the per-datatype encoding config - #1070

Open
ColinLeeo wants to merge 3 commits into
developfrom
compat-matrix-table-model
Open

ColinLeeo wants to merge 3 commits into
developfrom
compat-matrix-table-model

Conversation

@ColinLeeo

Copy link
Copy Markdown
Contributor

Follow-up to #1068.

What changed

  • Table model only. The cross-language matrix now exercises only the table
    model: the tree-model rows are dropped and both completeness tests
    (specMatchesJavaCapabilities, SpecMatchesCppCapabilities) enumerate only
    Model.TABLE. Type × encoding × compression coverage is unchanged, and the
    rle_overflow rows are kept.

  • Complete the per-datatype encoding config. set_datatype_encoding
    accepts the full set the encoder/decoder factories support: BOOLEAN RLE and
    FLOAT/DOUBLE RLE + GORILLA_V1. Previously
    set_tsfile_config(float_encoding_type_=RLE) and the DOUBLE equivalent failed
    with E_NOT_SUPPORT.

  • Cover TIMESTAMP and BLOB. get_datatype_encoding returned PLAIN for
    TIMESTAMP and BLOB instead of the INT64/STRING slot they share with their base
    types; it now maps TIMESTAMP → INT64 and BLOB → STRING, matching
    get_value_encoder. set_datatype_encoding also routes BLOB through the
    STRING/TEXT slot and rejects VECTOR/UNKNOWN with E_NOT_SUPPORT instead of
    silently ignoring them.

Testing

  • CWrapperTest.DatatypeEncodingConfigCoversTimestampAndBlob (new)
  • EncodingCompressionCompatibilityTest.SpecMatchesCppCapabilities
  • EncodingCompressionCompatibilityTest.FloatingRleOverflowPages
  • Java EncodingCompressionCompatibilityTest#specMatchesJavaCapabilities

The default-encoding config rejected BOOLEAN RLE, FLOAT/DOUBLE RLE and
FLOAT/DOUBLE GORILLA_V1 even though the encoder/decoder factory supports
them, so e.g. Python's set_tsfile_config(double_encoding_type_=RLE) failed
with E_NOT_SUPPORT. Align the config's allow-list with the factory (and with
Java's TYPE_SUPPORTED_ENCODINGS).
get_datatype_encoding returned PLAIN for TIMESTAMP and BLOB instead of the
INT64/STRING slot they share with their base types, so a table-model column of
those types read back a wrong default encoding. Align the getter with
set_datatype_encoding and get_value_encoder: TIMESTAMP maps to int64 and BLOB
maps to string. set_datatype_encoding also silently ignored BLOB, VECTOR and
UNKNOWN; it now routes BLOB through the STRING/TEXT slot and rejects VECTOR and
UNKNOWN with E_NOT_SUPPORT.
The table model is now the mainstream write model, so the compatibility spec
drops its tree-model rows and both completeness tests enumerate only the table
model. Coverage is unchanged: every (type, encoding, compression) combination is
still generated at all four scales, and the rle_overflow rows are kept.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.31%. Comparing base (d0a0bb4) to head (f47b556).

Files with missing lines Patch % Lines
cpp/src/common/global.h 80.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1070      +/-   ##
===========================================
+ Coverage    66.30%   66.31%   +0.01%     
===========================================
  Files          777      777              
  Lines        56372    56375       +3     
  Branches      9049     9049              
===========================================
+ Hits         37379    37387       +8     
+ Misses       17341    17336       -5     
  Partials      1652     1652              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants