WIP: DO NOT REVIEW: Benchmark enabling parquet filter pushdown by default with one-shot row group I/O - #24426
Draft
alamb wants to merge 6 commits into
Draft
WIP: DO NOT REVIEW: Benchmark enabling parquet filter pushdown by default with one-shot row group I/O#24426alamb wants to merge 6 commits into
alamb wants to merge 6 commits into
Conversation
… pushdown filters Adds `datafusion.execution.parquet.progressive_io` (default `false`). When filter pushdown is enabled, the parquet decoder normally fetches data progressively: the columns for each filter predicate first, then (after the filters are evaluated) the remaining projected columns for the rows that passed. With `progressive_io = false` (the default), all column chunks a row group could need (filter and projection columns) are instead fetched with a single I/O request per row group -- the same I/O pattern used when `pushdown_filters` is disabled. Filter evaluation itself is unchanged. With `progressive_io = true`, progressive fetching is used when it could actually reduce bytes read (the file has an offset index); files without an offset index still use one request per row group. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Flips `datafusion.execution.parquet.pushdown_filters` default to `true`. `progressive_io` remains `false`, so the I/O pattern stays one request per row group, the same as before this change. Benchmarking prototype -- not intended for merge as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
documentation
Contributor
Author
|
run benchmark clickbench_partitioned |
Contributor
Author
|
run benchmark clickbench_partitioned |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (79dd91d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (79dd91d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
env:
SIMULATE_LATENCY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing alamb/pushdown_by_default (79dd91d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing alamb/pushdown_by_default (79dd91d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
env:
SIMULATE_LATENCY: "true"CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
|
🤯 |
Contributor
|
Oh wait this is just mostly filter pushdown itself I guess |
Contributor
Author
Yes, exactly. However, the results are very close to not regressing with filter pushdown I think a few more tweaks and we'll have it |
Contributor
Author
|
run benchmark clickbench_partitioned |
Contributor
Author
|
run benchmark clickbench_partitioned |
…ter_mode) Ports zhuqi-lucas's unmerged PR apache#23420 onto this branch: adds `datafusion.execution.parquet.pushdown_filter_mode` (`auto` (default) / `always` / `heuristic`). In `auto`/`heuristic` mode, filter pushdown is declined for scans whose projection contains fewer than 3 columns not referenced by the filter, where RowFilter overhead tends to outweigh the decode it saves (e.g. ClickBench Q10/Q11/Q40). Dynamic (TopK) filters are never gated. Declined scans keep the filter in a FilterExec above the scan and still use the predicate for stats/bloom/page-index pruning. Differences from apache#23420 as-authored: proto tag 40 (38 is now taken by max_in_list_size), conversions added to proto-models and datasource-parquet file_format (proto conversion code moved since the PR was written), and one stale expected plan refreshed. Co-authored-by: Qi Zhu <821684824@qq.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Amazing |
Contributor
Author
|
FYI: I have pushed 026fb7d which ports the narrow-projection pushdown gate ( The two benchmark requests above from 15:21/15:23 UTC were still queued at push time, so check the |
Contributor
Author
|
run benchmark clickbench_partitioned |
Contributor
Author
|
run benchmark clickbench_partitioned |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (026fb7d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (026fb7d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
env:
SIMULATE_LATENCY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing alamb/pushdown_by_default (026fb7d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing alamb/pushdown_by_default (026fb7d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
env:
SIMULATE_LATENCY: "true"CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmark clickbench_partitioned |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (026fb7d) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (106653a) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (106653a) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
env:
SIMULATE_LATENCY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing alamb/pushdown_by_default (106653a) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing alamb/pushdown_by_default (106653a) to 00eba79 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
env:
SIMULATE_LATENCY: "true"CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
Author
Status updateThis PR now carries three logical changes (see description):
It is not clear to me if the refinement is necessary (claude code came up with it, but it may not be accurate) Next stepsBuilding Then we'll see if we can reproduce the reported slowdowns and if so we'll profile them to figure out what is going on Questions to answer: |
Contributor
Author
Local Q25 experiment: the regression is real, and the dynamic-only refinement makes it worseTo take benchmark-runner noise out of the picture, I built Each cell below is 10 runs × 3 interleaved rounds (30 runs) after 2 warmups; It is reproducable -- Q25:
SELECT "SearchPhrase" FROM hits WHERE "SearchPhrase" <> '' ORDER BY "SearchPhrase" LIMIT 10;
Conclusions:
Decision: reverting The next step is profiling the ~20–30ms ON-vs-OFF delta that exists on every binary for Q25 and figure out where the difference is |
This reverts commit 106653a.
Contributor
Author
|
Update: |
Contributor
Thanks @alamb, great work — following the Q25 dig here, a precise DataFusion-side gate that stacks with the arrow-rs work, in case it's useful: The row-level RowFilter is pure overhead in exactly one shape: when the (dynamic) filter column is the sole projected column. Then there's no "other column" for the selection to save a decode on; the filter column must be fully decoded to evaluate the threshold regardless; and for a high-cardinality key like That's a sharper predicate than "< 3 non-filter columns": decline only the row-level RowFilter (keep RG-level dynamic pruning) when Orthogonal to the selection-construction speedups in apache/arrow-rs#10776 — if those land this becomes unnecessary; until then it's a zero-risk way to drop the Q25 regression specifically. |
Contributor
Author
Thanks @zhuqi-lucas - I think I had claude try this - it is what is listed as
And it didn't seem to work (though perhaps it wasn't implemented correctly) |
Contributor
Thank you @alamb for the info , i can try to benchmark it locally first. Updated, tested locally, you are right. |
Contributor
Author
|
I am now going to build and profile the following combinations
|
Conflicts resolved: - row_filter.rs: main refactored build_row_filter into prebuild_row_filter_candidates / row_filter_from_prebuilt; the filter mask union for one-shot I/O is now computed from the prebuilt candidate list (new prebuilt_filter_mask helper + RowFilterContext::filter_mask) - opener/mod.rs: one_shot_ranges computation rebased onto main's InitialDecoderState block; filter mask sourced from RowFilterContext - push_decoder.rs: PushDecoderStreamState keeps both one_shot_ranges and main's new fully-matched toggle fields - object_store_access progressive_io snapshot updated: main's fully-matched RowFilter skip removes one request for the fully matched row group - dynamic_row_group_pruning.slt: main's new fully-matched suppression test pinned to pushdown_filter_mode=always (it projects only the filter column, which the narrow-projection gate would decline) - dynamic_filter_pushdown_config.slt: expected metrics updated for row_filter_skipped_fully_matched Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
run benchmark clickbench_partitioned |
Contributor
Author
|
run benchmark clickbench_partitioned |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (1eb7f67) to a38bb10 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
env:
SIMULATE_LATENCY: "true"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/pushdown_by_default (1eb7f67) to a38bb10 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing alamb/pushdown_by_default (1eb7f67) to a38bb10 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing alamb/pushdown_by_default (1eb7f67) to a38bb10 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
env:
SIMULATE_LATENCY: "true"CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
Contributor
Author
Local Q25 resultsI ran five Q25: SELECT "SearchPhrase" FROM hits WHERE "SearchPhrase" <> '' ORDER BY "SearchPhrase" LIMIT 10;
So the conclusion is that the pr from @hhhizzz makes a diffrence: |
Contributor
Author
|
I reran the results and 10852 is looking good |
This was referenced Aug 31, 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
4 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.
Which issue does this PR close?
Note: this is a benchmarking prototype and is NOT intended for merge or review.
Stacked on #24395 (the
progressive_iooption).Rationale for this change
Measure whether enabling
pushdown_filtersby default can now match or beat today's ClickBench performance when the I/O pattern is kept the same as today (one request per row group), i.e. withprogressive_io = false(the default introduced in #24395).What changes are included in this PR?
The
progressive_iocommit from WIP: NOT READY FOR REVIEW: Add parquetprogressive_iooption to keep one-shot row group I/O with pushdown filters #24395Flips the default of
datafusion.execution.parquet.pushdown_filterstotrue(progressive_iostaysfalse, so scans still fetch each row group's data in a single request)Ports the narrow-projection pushdown gate from feat(parquet): Enable Parquet
filter_pushdownby default, with heurstic fallback when projecting few non-filter columns #23420 (thanks @zhuqi-lucas): newdatafusion.execution.parquet.pushdown_filter_modeoption (auto(default) /always/heuristic). Inautomode, pushdown is declined for scans projecting fewer than 3 non-filter columns, whereRowFilteroverhead tends to dominate; dynamic (TopK) filters are never gated.Are these changes tested?
Benchmark runs only (
clickbench_partitioned, with and without simulated object store latency); results to be posted below. Other tests that encode the pushdown-off default are expected to fail and are not being fixed in this prototype.Are there any user-facing changes?
Not intended for merge as-is.