csun5285 opened a new pull request, #68608:
URL: https://github.com/apache/doris/pull/68608
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
Every block a segment writer wrote first went through OlapBlockDataConvertor,
which copied each column into a storage-shaped buffer: slices for strings,
CHAR values zero-padded to their declared length, serialized HLL, BITMAP,
QUANTILE_STATE and AGG_STATE objects, repacked V1 DATE, DATETIME and
DECIMALV2 values, and a separate null map. Column writers, page builders and
every index writer then read that buffer back through a type-erased
`const void*` and a count. So the write path copied every block once more,
and how a FieldType is stored was decided in several places at once: the
converter, core's PrimitiveTypeConvertor (used by the key coder, RowCursor
and
the inverted index reader), the two bloom filter probes and the zone map,
each
re-reading raw pointers.
This change removes the converter, and writers consume
`(const IColumn&, row_pos, n)` directly:
- `StorageLayout<FieldType>` (storage/storage_layout.h) is the one
compile-time table of how each FieldType is stored: `to_storage` and
`to_primitive` for a value, `column_to_storage` and `storage_to_column`
for a
run of rows, and `storage_at` for one row. The write and read sides share
it,
and core no longer knows storage types.
- Page builders take a column range. Fixed-width builders copy the rows as
stored values, in one copy when their bytes already are; string builders
read
the ColumnString. ScalarColumnWriter walks null runs in one place
(`for_each_null_run`), and the array, map, struct and variant writers feed
their children by row range.
- The zone map, bloom filter, inverted (CLucene and SNII), ANN and BKD index
writers, the indexed column writer and the primary key index take columns;
the rows an index sees for an ARRAY column are staged in one place
(`array_index_input`).
- RowKeyEncoder encodes short keys, primary keys and their sequence and rowid
suffixes from the block's own columns at absolute row numbers.
- JSONB values are still checked before they are written
(`admit_storage_rows`).
- Tests that fed writers raw pointers build columns instead; index writer
tests share be/test/storage/index/index_writer_feed.h.
CHAR is no longer zero-padded in data pages, dictionary pages, zone maps,
bloom filters or inverted indexes; only the key encodings pad it, through
`StorageLayout<CHAR>::append_padded`, which RowCursor also uses for its seek
keys. Segments written before this change still read the same: the CHAR page
pre-decoder already strips the padding of data and dictionary pages, a zone
map
bound of a CHAR column is already cut at its first '\0', and CLucene already
cuts an indexed value at its first '\0'. Only a bloom filter cannot tell
padded
hashes from unpadded ones, so CHAR predicates no longer prune with bloom
filters. The SegmentFlusherFormatTest goldens that change are those of the 32
cases with a CHAR column; the other 41 cases keep their bytes.
Performance, with the benchmarks this change adds to be/benchmark, against
the
same rows written by master:
- BM_SegmentFlushSingleBlock, a load's memtable flush: one 1M-row key-sorted
block per SegmentFlusher::flush_single_block. Wall clock over 10M rows,
6 samples each on a quiet machine, and user-space instructions of the whole
process over 5M rows, relative to master:
- DUP keys: 0.966x, -271M instructions
- UNIQUE merge-on-write: 0.986x, +167M instructions
- merge-on-write with a CHAR key: 0.966x, -32M instructions
- DUP keys with an indexed ARRAY: 0.988x, -706M instructions
- BM_SegmentFlushDupKeys, SegmentCreator::add_block, the path of push load,
horizontal compaction and schema change: 0.988x wall clock over 20M rows,
-275M instructions over 5M rows.
The page builders and the dictionary page add each row through inlined
helpers
(`ALWAYS_INLINE` on the per-row `_add_one`, `_add_dict_coded` and the lambdas
that forward to them, a single-cell `add_cell` for dictionary codes, a final
BinaryDictPageBuilder), the zone map takes the min and max of a string run
with `std::ranges::minmax`, and per-row casts skip the release-build type
check, which is what brings these paths level with or under master.
### Release note
CHAR columns are stored without their zero padding outside the key encodings,
so their data pages and zone maps get smaller. Query results do not change.
### Check List (For Author)
- Test: Unit Test, Regression test, Manual test (benchmark)
- New unit tests: PageFormatTest pins the on-disk bytes of every page
encoding but FOR (86 cases, goldens under
be/test/storage/test_data/page_format), StorageLayoutTest and
PageBuilderColumnTest.
- Passed on this revision: RowKeyEncoderTest, KeyCoderTest,
KeyCoderTraitsTest, StorageLayoutTest, PageBuilderColumnTest,
KeyProbeTest, RowCacheProbeTest, FixedPartialUpdateTest,
FlexiblePartialUpdateTest, PageFormatTest, SegmentFlusherFormatTest and
SegmentFlusherTransformFormatTest (strict golden comparison),
VerticalSegmentWriterWritePathsTest, VerticalSegmentWriterMowCheckTest,
HistoricalRowRetrieverTest, ColumnZoneMapTest, BinaryDictPageTest,
BinaryPlainPageV2Test, BinaryPlainPageV3Test, EncodingInfoTest,
ColumnReaderTest, InvertedIndexWriterTest and
PhraseCandidatePushdownBench (307 tests).
- Regression test: added
unique_with_mow_p0/partial_update/test_partial_update_v1_date_zone_map.
It and the datev1, datetimev1, partial_update and flexible suites (73)
passed on a 2026-09-26 build of this branch; they were not rerun on
this revision.
- Benchmarks: some of the numbers above were measured on earlier
revisions of this branch; every change after them only removes work
(`add_cell` without a count, per-row casts without the release-build
check).
- Behavior changed: Yes. CHAR is stored unpadded outside the key encodings,
and CHAR predicates no longer prune with bloom filters.
- Does this need documentation: No
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
