[improvement](parquet) Fuse nested sparse selection planning - #66263
Merged
Gabriel39 merged 2 commits intoJul 30, 2026
Conversation
### What problem does this PR solve? Issue Number: N/A Related PR: apache#65674 Problem Summary: Parquet V2 repeated leaves rebuilt parent-row filters, null runs, ancestor-null state, leaf selections, and compacted level streams in separate passes. Fuse these operations into one pass while preserving nested parent shape and physical leaf cursor semantics. Release microbenchmarks improve CPU time by 1.79x to 2.27x across 1%, 10%, and 50% parent-row survivor rates. ### Release note Improve Parquet V2 sparse reads of nested and repeated columns by reducing selection reconstruction work. ### Check List (For Author) - Test: Unit Test and manual Release microbenchmark - Behavior changed: No - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 29612 ms |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review completed for head 5239636ff6340407a4d5abedf84fa0b11dcedb9a.
Four actionable review issues are reported inline: the production nested partial-filter path lacks end-to-end boundary coverage; the new microbenchmark does not independently validate its output; the before/after performance claim lacks a reproducible baseline/artifact record; and the refreshed benchmark guide retains a stale reader-case count.
Critical checkpoint conclusions:
- Goal and correctness: The change is a focused one-pass fusion of parent filtering, leaf null classification, decoder selection, and level compaction. Main and independent risk-review differential models matched the old algorithm in 200,000 and 300,000 generated segmented cases, and the production cursor/shape trace found no implementation defect. The missing real-reader integration test still leaves the changed caller/page/complex-reconstruction boundary unproved.
- Scope, concurrency, and lifecycle: The patch is localized and introduces no shared concurrency, locks, threads, static initialization, ownership cycle, or configuration. Existing persistent reader scratch is reset and reused within the established row-group lifecycle.
- Data, NULL, and error semantics: Ancestor placeholders, leaf logical entries, encoded non-null payload, selected output, and conversion-failure null bits remain separately accounted; new status failures propagate. No transaction, persistence, storage-write, visibility, delete-bitmap, or FE-BE propagation path is involved.
- Compatibility and parallel paths: No wire/storage-format compatibility change is introduced. Identity and fully filtered inputs retain their existing paths; partial nested filters use the fused path. ARRAY/MAP/STRUCT sibling and parent-shape consumers were traced without finding another defect.
- Tests and results: Added helper tests cover local compaction and a nonzero segment start, but not the full physical reader/page/complex output boundary. The benchmark matrix count of 86 is consistent, while its correctness check and the guide's reader count are not. No local build or test was run because the authoritative review instructions prohibit builds in this runner.
- Performance and observability: Setup restoration is outside the timed region, the fused helper is the measured work, scratch is reused, and
_decode_null_map_timeattribution remains intact. The summarized speedup cannot be independently reproduced from the submitted revisions without the missing baseline patch/environment/raw data. - User focus: No additional user-provided review focus was supplied; the complete PR was reviewed.
Outcome: request changes for the four inline items. Apart from those coverage, benchmark-evidence, and documentation issues, no additional production correctness defect was substantiated.
| EXPECT_EQ(filter, nullptr); | ||
| } | ||
|
|
||
| TEST(NativeNestedSelectionTest, BuildsSelectionAndCompactsSurvivingParentLevels) { |
Contributor
There was a problem hiding this comment.
[P1] Exercise the fused path through a real nested reader
These new tests call init_nested() directly, but the production change also depends on _read_nested_column() supplying the right batch or continuation filter index, materialize_values() and the ancestor-only skip advancing the page cursor exactly, and ARRAY/MAP/STRUCT consumers rebuilding the compacted shape. A partial-filter bug in any of those boundaries can shift later values or corrupt offsets while both helper tests still pass. Please add an end-to-end Parquet reader case with a partially filtered batch, selected and rejected ancestor/leaf NULLs, a parent spanning pages, and follow-up output that verifies exact values, offsets, and null maps (including sibling shape for MAP/STRUCT).
| /*repeated_parent_def_level=*/2, | ||
| /*definition_level=*/3, &selected_nulls, &parent_filter, 0, | ||
| &ancestor_null_count); | ||
| if (!status.ok() || repetition_levels.size() != definition_levels.size() || |
Contributor
There was a problem hiding this comment.
[P2] Validate nested-selection output before timing it
These checks cannot catch the kernel keeping the wrong parents or emitting incorrect null/read runs: the two level vectors are resized together, and every source entry necessarily contributes to either num_values() or ancestor_null_count. The existing kernel benchmarks compare representative output with an independent oracle before timing. Please build the expected compacted repetition/definition levels, selected null map, filtered count, and four-way read sequence independently and compare them here before publishing timings.
| constexpr size_t KERNEL_ROWS = 1UL << 16; | ||
| constexpr size_t NESTED_VALUES_PER_ROW = 8; | ||
|
|
||
| inline void run_nested_selection_kernel(benchmark::State& state, const KernelScenario& scenario) { |
Contributor
There was a problem hiding this comment.
[P2] Make the before/after result reproducible
This benchmark and the fused implementation are introduced together, so the base revision cannot run ParquetKernel/nested_selection and the head exposes only the new path. The summary table therefore cannot be reproduced without an unsubmitted benchmark-only baseline. Per this directory's benchmark guide, please provide the exact baseline patch/commit and command, compiler/CPU/cache environment, and raw ABBA JSON (or retain selectable legacy and fused implementations in the harness) so the claimed 1.79x-2.27x result can be verified.
| ## Current validation record | ||
|
|
||
| The current expected registration counts are 228 decoder, 80 kernel, and 152 reader cases. A smoke | ||
| The current expected registration counts are 228 decoder, 86 kernel, and 152 reader cases. A smoke |
Contributor
There was a problem hiding this comment.
[P2] Keep the reader registration count in sync
This refreshed validation record still says ParquetReader has 152 cases, but ReaderMatrixCoversNullableSparseAndProjectionAxes now asserts reader_scenarios().size() == 167, and register_reader_benchmarks() registers every returned scenario. The list/count command above will therefore report 167 and contradict this review guide (the stale 152 also appears in the command comment and matrix description). Please update all three reader-count references to 167 while refreshing these totals.
Contributor
TPC-DS: Total hot run time: 177618 ms |
Contributor
ClickBench: Total hot run time: 24.76 s |
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 29268 ms |
Contributor
There was a problem hiding this comment.
Automated review result: no new actionable findings at head e8e84487770d72f498760d3bdeb9e1e91455218e.
Critical checkpoint conclusions:
- Goal and proof: the change replaces the partial-filter nested path with one pass that maps parent selection, classifies ancestor/leaf nulls, builds the four-way decoder plan, appends selected null bits, and compacts levels. Direct helper tests, an independent benchmark oracle, and the real MAP/STRUCT-with-LIST reader case prove the intended behavior.
- Scope and clarity: the production change is focused on one internal selector plus its native nested-reader call site. Benchmark, matrix, documentation, and tests are directly related.
- Data correctness and error handling: parent filter coordinates match the legacy path on first pages, V1 continuation fragments, later parents, and later batches. Selected/filtered content and NULL runs preserve logical and payload cursor counts; ancestor placeholders are skipped logically once; ARRAY/MAP/STRUCT receive the same compacted shape. Invalid bounds or inconsistent page/filter state return errors rather than continuing.
- Concurrency and lifecycle: native column readers and their scratch are synchronously owned by one row-group scheduler path. No new shared state, locks, static initialization, ownership cycle, or cleanup path is introduced.
- Compatibility and parallel paths: no storage format, FE/BE protocol, function symbol, configuration, persistence, transaction, or data-write contract changes. Partial filters use the fused path; unfiltered and fully filtered paths retain their existing behavior. Decoder encoding dispatch is unchanged.
- Performance and observability: the same Release binary retains legacy and fused cases, validates both against an independent source-level oracle before timing, and documents ABBA commands. The reported host load is disclosed, so the numbers are evidence of the same-binary delta rather than an isolated gate. Fused work remains attributed to the existing decode/null-map timing.
- Test coverage: registration counts are consistently 228 decoder, 92 kernel, and 167 reader cases. The reader fixture proves MAP key/value and LIST continuation pages, drives the selected wide parent through a partial filter, and verifies exact values, offsets, parent/leaf null maps, STRUCT sibling shape, and trailing cursor alignment. This review runner was explicitly review-only, so I did not run builds or tests; the PR records the author-run ASAN unit tests and Release benchmark cases.
Existing inline review concerns were rechecked against the current head: the real continuation-path test, independent oracle, selectable same-binary legacy/fused harness, ABBA instructions, and registration-count updates are now present.
User focus: no additional focus was provided; the complete PR was reviewed.
Review completion: Round 1 included two normal full-coverage reviews and a separate continuation/cursor risk review. All returned NO_NEW_VALUABLE_FINDINGS; every risk was resolved with code evidence, and the frozen inline comment set is empty.
Contributor
TPC-DS: Total hot run time: 176924 ms |
Contributor
ClickBench: Total hot run time: 24.78 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A
Related PR: #65674
Problem Summary:
Parquet V2 repeated leaves currently rebuild a parent-row filter into a level-entry filter, scan definition levels into null runs plus an ancestor-null hash set, build the leaf selection, and scan the levels again to compact surviving parent shapes. This fixed O(level entries) work is especially visible when only 1% or 10% of parent rows survive.
This change adds a single-pass nested selection builder. It maps parent filters to repeated entries, classifies ancestor and leaf nulls, builds the decoder selection, appends selected nulls, and compacts repetition/definition levels in place. Ancestor-null placeholders remain in the surviving parent shape but do not consume leaf selection entries. Unfiltered and fully filtered paths keep their existing behavior.
Correctness coverage
rep > 0) for MAP keys, MAP values, and LIST elements, so the fixture cannot silently regress to row-boundary-only pages.Reproducible microbenchmark
The benchmark retains the pre-fusion
impl_legacypath and the optimizedimpl_fusedpath in the same Release binary. Both use the same generated levels, filter, compiler, and process setup. The scenario matrix contains six shapes for each implementation (12 nested-selection cases total).Environment: Clang 20.1.8, Linux 5.14, Release build, Intel Xeon Platinum 8457C (2 sockets, 48 cores/socket, SMT enabled), pinned to logical CPU 71. CPU scaling was enabled. The host load averages at the four process starts were 535.98, 594.98, 610.33, and 640.04, so these CPU-time results demonstrate the same-binary delta but are not presented as an isolated performance gate.
Exact build and ABBA commands (three untimed
--benchmark_min_time=0.1swarmups were run first):Raw ABBA median CPU times from the four JSON outputs (milliseconds; speedup uses the mean of the two medians per implementation):
Verification
benchmark_testbuilt successfully and all 12 nested-selection cases completed without oracle errors.Release note
Improve Parquet V2 sparse reads of nested and repeated columns by reducing selection reconstruction work.
Check List (For Author)