[Parquet] Improve Auto RowSelection construction for scattered predicate results - #10852
[Parquet] Improve Auto RowSelection construction for scattered predicate results#10852hhhizzz wants to merge 4 commits into
Conversation
|
run benchmarks |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix/issue-10776-capped-auto-selection (d087f0e) to c62002b (merge-base) diff Run configurationrun benchmark sql_plannerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench sql_planner File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: `` Kubernetes messageFile an issue against this benchmark runner |
|
run benchmarks |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix/issue-10776-capped-auto-selection (d087f0e) to c62002b (merge-base) diff Run configurationrun benchmark sql_plannerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench sql_planner File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: `` Kubernetes messageFile an issue against this benchmark runner |
|
run benchmarks |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix/issue-10776-capped-auto-selection (d087f0e) to c62002b (merge-base) diff Run configurationrun benchmark sql_plannerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench sql_planner File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: `` Kubernetes messageFile an issue against this benchmark runner |
|
I'm not sure how to run whole bench in this PR, So I run them on my dev server, here's the main results: End-to-end benchmark resultsI ran three cyclic-order rounds against Arrow main using the same DataFusion integration harness. The first iteration of each query was excluded, and the table reports the cross-round median of steady-state query times.
Selected queries:
The candidate fixes the target Q25 regression, preserves the major pushdown wins such as ClickBench Q23, TPC-H Q18, and TPC-DS Q72, and reduces several other Main-ON regressions. It is not a pure monotonic improvement: regressions remain on queries such as ClickBench Q21 and TPC-DS Q14/Q50, and pushdown is still not universally faster than OFF. All query row counts matched across every arm and round. |
|
run benchmarks |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix/issue-10776-capped-auto-selection (4ea9518) to c62002b (merge-base) diff Run configurationrun benchmark sql_plannerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench sql_planner File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: `` Kubernetes messageFile an issue against this benchmark runner |
|
run benchmarks |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix/issue-10776-capped-auto-selection (4ea9518) to c62002b (merge-base) diff Run configurationrun benchmark sql_plannerBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench sql_planner File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: `` Kubernetes messageFile an issue against this benchmark runner |
|
run benchmark arrow_reader_clickbench |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing fix/issue-10776-capped-auto-selection (4ea9518) to c62002b (merge-base) diff Run configurationrun benchmark arrow_reader_clickbenchBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_reader_clickbench File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing fix/issue-10776-capped-auto-selection (4ea9518) to c62002b (merge-base) diff Run configurationrun benchmark arrow_reader_clickbenchCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
Thanks @hhhizzz, look good to me. This is a good improvement for the first-predicate path. One remaining limitation is that when the selection stays selector-backed, later predicates still fully materialize selectors via for the regression in TPC-DS Q14/Q50, maybe we can run the benchmark once again to check if this a noisy, because both Q14/Q50 are join-dominated, the pr should not affect so much. also i can help test, if you can share how to run the test . |
I have a bunch of local environment to compile the datafusion and arrow to do the bench. Let me run a few more time quickly. |
Thanks for pointing this out. I investigated the apparent regressions further. In a new four focused 10-iteration rounds, neither regression reproduced:
Profiles showed no new candidate hotspot, and the Q14/Q50 direct-output counters were zero. The earlier regressions were most likely caused by insufficient iterations and benchmark noise. I’ll treat that table as experimental context rather than PR-only evidence. |
5 participants
Which issue does this PR close?
ReadPlanBuilder::with_predicate_options#10776.Rationale for this change
For the first predicate, when there is no existing
RowSelection,ReadPlanBuilder::with_predicate_optionscurrently materializes all predicate results as RLE selectors.RowSelectionPolicy::Automay then decide that the selection is too fragmented, convert those selectors back to a bitmap, and use the mask strategy.For scattered selections such as ClickBench Q25, this creates a large temporary
Vec<RowSelector>even though the final strategy becomes certain as soon as the normalized run count crosses Auto's threshold.Keeping the first fragmented predicate mask-backed also benefits subsequent predicates: they use the existing mask construction path and mask conjunction instead of rebuilding another large selector vector.
What changes are included in this PR?
RowSelection::from_filters_autoconstructor that builds normalized selectors only while the selector strategy is still possible.auto_min_mask_runshelper between selector-backed strategy resolution, mask-backed strategy resolution, and capped construction. This keeps the strict comparison, threshold0/1, and saturating overflow behavior in one place.BooleanArrays.from_filterspath when no attainable run count can select Mask, avoiding a per-selector cutoff check for threshold0and1.from_filters_maskas the semantic mask constructor, including a single-filter fast path that reuses the filter'sBooleanBuffer.RowSelectionPolicy::Auto. Existing selections and explicit Mask/Selectors policies retain their existing paths.prepare_selection_for_page_skippingwhen there is no selection yet. There are no selection-driven pages to skip in that state, and resolving early would force Selectors before the first predicate result is available.Are these changes tested?
Yes.
Correctness coverage includes:
31/32run-length threshold boundaries and shared-helper boundary checks;BooleanArraysplits;Focused construction benchmark over 4,194,304 rows, split into 512
BooleanArrays of 8,192 rows each (median of three Criterion rounds):An unconditional mask-first implementation was rejected because it regressed selector-friendly run-128 by 30.7% and sparse input by 107.6%.
Three paired async-reader rounds with PageIndex disabled showed:
The focused construction harness was kept separate in accordance with the repository guidance for benchmark additions. These results isolate selection construction and async reader behavior; they are not presented as a full ClickBench Q25 wall-time measurement.
Local validation:
cargo fmt --all -- --check: passedcargo test -p parquet --lib -- --skip file::writer::tests::test_int96_interop: 1,305 passed, 0 failed, 1 filteredcargo test -p parquet --test arrow_reader --features async -- --skip bad_data::test_invalid_files: 123 passed, 0 failed, 1 ignored, 1 filteredcargo clippy -p parquet --all-targets --all-features -- -D warnings: passedThe two filtered tests require fixtures absent from the local
parquet-testingcheckout (int96_timestamp_order.parquetand thebad_data/variantsfixture). The PR's GitHubparquettest, compilation, and Clippy checks pass without filtering.Are there any user-facing changes?
No public interface changes. The new constructors and cutoff helper are crate-private, and logical row-selection semantics and explicit policy behavior are unchanged.