Uh oh!
There was an error while loading. Please reload this page.
[improvement](be) Use plain encoding for V3 floating-point columns - #62649
Conversation
Thearas
commented
Apr 20, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
bobhan1
commented
Apr 21, 2026
run buildall |
hello-stephen
commented
Apr 21, 2026
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Apr 21, 2026
FE UT Coverage ReportIncrement line coverage |
bobhan1
commented
Apr 23, 2026
run external |
bobhan1
commented
Apr 23, 2026
run feut |
bobhan1
commented
Apr 23, 2026
run p0 |
hello-stephen
commented
Apr 23, 2026
FE UT Coverage ReportIncrement line coverage |
bobhan1
commented
Apr 24, 2026
run external |
a6f0314 to
5275234Comparebobhan1
commented
Apr 24, 2026
run buildall |
hello-stephen
commented
Apr 24, 2026
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Apr 24, 2026
FE Regression Coverage ReportIncrement line coverage |
hello-stephen
commented
Apr 24, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
5275234 to
5394bd3Comparebobhan1
commented
Aug 17, 2026
/review |
bobhan1
commented
Aug 17, 2026
run buildall |
There was a problem hiding this comment.
Review opinion: changes requested
One blocking test-integration issue remains: the V3 FLOAT/DOUBLE default switch leaves checked-in SegmentFlusher golden segments stale. See the inline comment for the affected scope and regeneration requirement.
Critical checkpoint conclusions:
- Goal and scope: The production change correctly selects PLAIN for V3 FLOAT/DOUBLE and retains BIT_SHUFFLE for V2. The code and focused map tests are small and clear, but the required golden artifacts are incomplete.
- Data correctness and compatibility: Each writer persists a concrete
ColumnMetaPB.encoding, and readers dispatch from that stored value rather than recomputing a default. Both FLOAT/DOUBLE PLAIN and BIT_SHUFFLE codecs already exist, so old BIT_SHUFFLE segments and new PLAIN segments can coexist across rolling upgrades. - Parallel paths: Horizontal, vertical, nested, Variant, load, compaction, and schema-change paths preserve the owning tablet storage format and route encoding selection through the same resolver. No missed production path remains.
- Concurrency, lifecycle, and configuration: The resolver maps are immutable after initialization. This PR adds no lock, thread, lifecycle, static-initialization, dynamic-config, or new FE/BE variable concern.
- Persistence and writes: Only newly written V3 page selection changes; transactionality, atomicity, tablet visibility, and metadata read/write protocols are otherwise unchanged.
- Tests and results: The changed unit tests directly lock the V2/V3 map entries, and the regression performs V2/V3 write/read smoke coverage. However, normal
SegmentFlusherFormatTestexecution byte-compares against stale checked-in V3 fixtures, so six cases fail and ten.datblobs need regeneration. The author's reported BE-UT filter covered onlyEncodingInfoTest.*; no builds or tests were run in this review-only runner. - Observability and protocols: No new protocol symbol, storage enum, log, metric, or observability requirement is introduced.
- Performance: The change removes BIT_SHUFFLE work from new V3 FLOAT/DOUBLE pages without adding per-row work or allocations; no independent performance benchmark was run here.
- User focus: No additional user-provided focus was supplied.
Review completion: the review converged after two full-coverage and risk-focused rounds. One finding is accepted inline; the weaker regression-result-oracle candidate was dismissed with concrete code evidence.
| _set_v3_default<FieldType::OLAP_FIELD_TYPE_UNSIGNED_INT, BIT_SHUFFLE>(); | ||
| _set_v3_default<FieldType::OLAP_FIELD_TYPE_FLOAT, BIT_SHUFFLE>(); | ||
| _set_v3_default<FieldType::OLAP_FIELD_TYPE_DOUBLE, BIT_SHUFFLE>(); | ||
| _set_v3_default<FieldType::OLAP_FIELD_TYPE_FLOAT, PLAIN_ENCODING>(); |
There was a problem hiding this comment.
Please regenerate the affected V3 SegmentFlusher goldens with this default change. Normal SegmentFlusherFormatTest execution byte-compares generated segments with the checked-in files, and this PR updates none of them. Besides the two all_scalar_values_v3_* cases, four complex_v3_* cases materialize nested.y:3.5 as a DOUBLE subcolumn, so six case directories fail and ten .dat blobs differ. Those fixtures were generated while V3 FLOAT/DOUBLE used BIT_SHUFFLE; this line changes their stored ColumnMetaPB.encoding and page bytes to PLAIN. Please regenerate all six affected case directories and run the full SegmentFlusherFormatTest suite.
5394bd3 to
a4a7cdaComparebobhan1
commented
Aug 18, 2026
run buildall |
bobhan1
commented
Aug 18, 2026
/review |
There was a problem hiding this comment.
Review opinion: changes requested
No new inline finding survived review on the current head. The existing blocking thread remains fully applicable: #62649 (comment). The authoritative changed-file list still contains no regenerated SegmentFlusher fixtures, so six V3 case directories and ten .dat blobs remain stale.
Critical checkpoint conclusions:
- Goal and scope: The production change is small and correctly selects PLAIN for V3 FLOAT/DOUBLE while preserving V2 BIT_SHUFFLE. The focused map tests lock that split, but the checked-in physical artifacts required by the normal test suite are incomplete.
- Data correctness and compatibility: Writers persist a concrete
ColumnMetaPB.encoding, and readers dispatch from that stored value rather than recomputing a default. Both FLOAT/DOUBLE PLAIN and BIT_SHUFFLE codecs predate this change, so historical V3 BIT_SHUFFLE segments and new V3 PLAIN segments remain readable across rolling upgrades. - Parallel paths: Horizontal, vertical, nested ARRAY/MAP/STRUCT, Variant materialization/compaction, direct load, rowset compaction, and schema-change writers preserve the owning tablet storage format and route metadata through the same resolver. No missed production path remains.
- Concurrency, lifecycle, and configuration: The resolver maps are immutable after initialization. This PR adds no thread, lock, lock-order, static-initialization, lifecycle, dynamic-configuration, or FE/BE variable concern.
- Persistence and write semantics: Only the encoding chosen for newly written V3 FLOAT/DOUBLE pages changes. Transactionality, atomicity, tablet-version visibility, EditLog behavior, and crash recovery protocols are otherwise untouched.
- Tests and results: The unit tests directly pin V3 PLAIN and V2 BIT_SHUFFLE, existing page tests cover both codecs, and the regression executes V2/V3 FLOAT/DOUBLE insert/read paths. However, normal
SegmentFlusherFormatTestexecution byte-compares against the unchanged V3 fixtures identified in the existing thread. No builds or tests were run in this review-only runner. - Observability and protocols: No new protocol symbol, storage enum, log, metric, or additional observability requirement is introduced.
- Performance: The change removes BIT_SHUFFLE work from new V3 FLOAT/DOUBLE pages without adding per-row work or allocations; no independent performance benchmark was run here.
- User focus: No additional user-provided focus was supplied.
- Review completion: Round 1 converged. Both normal full-review subagents and the separate risk-focused subagent returned
NO_NEW_VALUABLE_FINDINGS; every risk was independently resolved, the known golden issue was deduplicated against the existing thread, and the final comment set contains zero new inline comments.
hello-stephen
commented
Aug 18, 2026
TPC-H: Total hot run time: 17161 ms |
hello-stephen
commented
Aug 18, 2026
TPC-DS: Total hot run time: 80785 ms |
a4a7cda to
e92feedComparebobhan1
commented
Aug 18, 2026
/review |
bobhan1
commented
Aug 18, 2026
run buildall |
bobhan1
commented
Aug 18, 2026
run beut |
There was a problem hiding this comment.
Review opinion: changes requested
The current head fixes the scalar portion of the existing golden-fixture thread, but the thread remains blocking: #62649 (comment). All four V3 complex cases still have stale checked-in output. Specifically, all four complex segment_0.dat files and both non-nullable complex segment_1.dat files still encode the materialized nested.y:3.5 DOUBLE subcolumn with BIT_SHUFFLE, while the current writer selects PLAIN. Those six blobs are absent from the changed-file list, so the full SegmentFlusherFormatTest producer set is still inconsistent. No new inline comment is added because the existing thread already covers the mechanism, four-case scope, and required full regeneration.
Critical checkpoint conclusions:
- Goal and scope: The production change is small and correctly selects PLAIN for V3 FLOAT/DOUBLE while retaining BIT_SHUFFLE for V2. The direct map tests and regenerated scalar metadata prove that split, but the required complex golden artifacts remain incomplete.
- Data correctness and write semantics: Writers persist a concrete
ColumnMetaPB.encoding, and readers dispatch from that stored value. FLOAT/DOUBLE PLAIN and BIT_SHUFFLE codecs already coexist, and active V3 typed-DOUBLE regression coverage verifies exact values through the PLAIN Segment reader. Transactionality, atomicity, tablet visibility, crash recovery, and delete-bitmap behavior are otherwise untouched. - Compatibility and parallel paths: Horizontal, vertical, nullable, nested ARRAY/MAP/STRUCT, Variant materialization, load, compaction, and schema-change paths preserve the owning tablet storage format and use the common resolver. Historical BIT_SHUFFLE segments and new PLAIN segments remain readable across rolling upgrades; no parallel production path was missed.
- Concurrency, lifecycle, configuration, and special conditions: The resolver maps are immutable after initialization. This PR adds no thread, lock, lock-order, lifetime, static-initialization, dynamic-config, conditional-guard, or memory-ownership concern.
- Persistence, protocols, and cross-process variables: The existing Segment encoding enum is persisted per column; no EditLog path, FE/BE variable, function symbol, storage-format enum, RPC field, or mixed-version protocol contract is added.
- Tests and results: The focused resolver tests lock V2 and V3 defaults, and the four regenerated scalar blobs cover horizontal/vertical plus nullable/non-nullable V3 writes. The author's reported SegmentFlusher filter exercised only the scalar producer, so it could not catch the six omitted complex blobs. The changed regression executes V2/V3 read smoke coverage; its discarded results are not a distinct defect because an active V3 typed-DOUBLE regression supplies an exact PLAIN-reader oracle through the same templated fixed-width path. No builds or tests were run in this review-only runner.
- Observability: Existing writer VLOG output records the concrete encoding. No new log, metric, or tracing surface is needed for this two-entry policy change.
- Performance: Selecting PLAIN removes BIT_SHUFFLE work for newly written V3 FLOAT/DOUBLE pages and adds no per-row branch, scan, or allocation. No independent performance benchmark was run here.
- User focus: No additional user-provided review focus was supplied.
- Review completion: The review converged after two full-coverage and risk-focused rounds. All Round 2 subagents returned
NO_NEW_VALUABLE_FINDINGS; every candidate was independently verified and either dismissed with code/test evidence or deduplicated against the existing inline thread. The final comment set intentionally contains zero new inline comments.
hello-stephen
commented
Aug 18, 2026
TPC-H: Total hot run time: 17250 ms |
hello-stephen
commented
Aug 18, 2026
TPC-DS: Total hot run time: 82911 ms |
hello-stephen
commented
Aug 18, 2026
ClickBench: Total hot run time: 14.36 s |
e92feed to
24943ccCompare### What problem does this PR solve? Issue Number: None Related PR: apache#63622 Problem Summary: Segment V3 continued to select BIT_SHUFFLE for FLOAT and DOUBLE columns. The original implementation required a tablet-schema flag propagated through proto, FE, Cloud, and BE writers. After apache#63622 split encoding selection into explicit V2 and V3 maps and persisted storage_format in TabletSchema, that propagation chain is obsolete. Express the behavior directly in the V3 default map, preserve the V2 BIT_SHUFFLE default for compatibility, and refresh all affected scalar and materialized Variant Segment goldens. ### Release note Segment V3 now uses PLAIN encoding by default for FLOAT and DOUBLE columns. Segment V2 behavior is unchanged. ### Check List (For Author) - Test: Unit Test - `./run-be-ut.sh --run --filter='SegmentFlusherFormatTest.*KeepTheirSegmentBytes:SegmentFlusherTransformFormatTest.PartialUpdateAndRowBinlogPathsKeepTheirSegmentBytes' -j100` (6 tests passed in two generation runs; all 164 generated Segment files were byte-identical between runs) - `./run-be-ut.sh --run --filter='EncodingInfoTest.*:SegmentFlusherFormatTest.*:SegmentFlusherTransformFormatTest.*' -j100` (17 tests passed against the checked-in goldens) - `build-support/clang-format.sh` - `build-support/check-format.sh` - `git diff --check` - Behavior changed: Yes. Newly written V3 FLOAT and DOUBLE pages use PLAIN instead of BIT_SHUFFLE; V2 remains BIT_SHUFFLE. - Does this need documentation: No
24943cc to
276cfc4Comparebobhan1
commented
Aug 26, 2026
run buildall |
bobhan1
commented
Aug 26, 2026
/review |
There was a problem hiding this comment.
Review opinion: no new blocking issue found; the current head is ready from the code-review perspective.
Critical checkpoint conclusions:
- Goal and proof: the V3 default map now selects numeric
PLAIN_ENCODINGfor FLOAT and DOUBLE, while V2 remainsBIT_SHUFFLE. The focused assertions and exhaustive V2/V3 expectation tables lock both behaviors. - Scope and clarity: the source change is the minimal two-entry policy update enabled by the existing split default maps; it does not reintroduce schema flags or cross-layer propagation.
- Data correctness and compatibility: every writer persists the resolved enum in
ColumnMetaPB.encoding, and readers dispatch from that stored enum. Both encodings were already registered at the merge base, so old readers accept new PLAIN V3 pages and new readers accept existing BIT_SHUFFLE V3 pages during rolling upgrades. - Parallel paths: horizontal, vertical, nested complex-child, auxiliary child, and materialized Variant subcolumn writers all propagate the tablet storage format through the same resolver. The scalar and Variant fixtures exercise both writer families, nullability, Bloom/no-Bloom, and compressed/uncompressed cases.
- Floating-point and index semantics: numeric PLAIN copies fixed-width bytes verbatim, preserving finite values, NaN payloads, infinities, and signed zero. Nullmaps, logical page counts, zone maps, Bloom filters, ordinals, and page pointers remain independent of the physical transform.
- Concurrency, lifecycle, configuration, protocol, transaction, and persistence: no mutable runtime state, lock, ownership, dynamic config, FE/BE variable, transaction, visibility, delete-bitmap, or crash-recovery path is changed.
- Error handling, memory, performance, and observability: unsupported encodings still fail through checked
Status; no allocation ownership or tracking changes. PLAIN removes the bitshuffle transform and the regenerated samples are smaller. Broader performance benchmarking is a product tradeoff, but no correctness or severe performance regression was substantiated. Existing VLOG context is sufficient and no new metric is needed. - Tests and results: the current head includes exactly the ten regenerated blobs requested by the existing inline thread. Independent head/base decoding found only the intended four scalar FLOAT/DOUBLE pairs and six Variant DOUBLE pages recoded from 6 to 2; all other semantic metadata and referenced pages remain unchanged apart from expected size/offset relocation. The prior concern is therefore addressed. The PR reports deterministic two-pass generation of all 164 segments and a 17-test EncodingInfo/SegmentFlusher run; I did not rerun builds or tests because this review environment explicitly forbids builds.
- User focus: no additional focus was supplied; the full PR was reviewed.
No inline comments are proposed.
hello-stephen
commented
Aug 26, 2026
TPC-H: Total hot run time: 17036 ms |
hello-stephen
commented
Aug 26, 2026
TPC-DS: Total hot run time: 85479 ms |
hello-stephen
commented
Aug 26, 2026
ClickBench: Total hot run time: 14.83 s |
hello-stephen
commented
Aug 26, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Aug 26, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Uh oh!
There was an error while loading. Please reload this page.
What problem does this PR solve?
Issue Number: None
Related PR: #63622
Problem Summary: Segment V3 continued to select
BIT_SHUFFLEforFLOATandDOUBLEcolumns. The original implementation added a tablet-schema flag and propagated it through proto, FE, Cloud, and BE writers. After #63622 split encoding selection into explicit V2 and V3 maps and persistedstorage_formatinTabletSchema, that propagation chain is obsolete. This PR expresses the behavior directly in the V3 default map while preserving the V2 default for compatibility.Changes:
PLAIN_ENCODINGas the V3 default forFLOATandDOUBLE.BIT_SHUFFLE.Release note
Segment V3 now uses PLAIN encoding by default for FLOAT and DOUBLE columns. Segment V2 behavior is unchanged.
Check List (For Author)
Test
./run-be-ut.sh --run --filter='SegmentFlusherFormatTest.*KeepTheirSegmentBytes:SegmentFlusherTransformFormatTest.PartialUpdateAndRowBinlogPathsKeepTheirSegmentBytes' -j100(6 tests passed in two generation runs; all 164 generated Segment files were byte-identical between runs)./run-be-ut.sh --run --filter='EncodingInfoTest.*:SegmentFlusherFormatTest.*:SegmentFlusherTransformFormatTest.*' -j100(17 tests passed against the checked-in goldens)build-support/clang-format.shbuild-support/check-format.shgit diff --checkBehavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)