feat(parquet): Enable Parquet filter_pushdown by default, with heurstic fallback when projecting few non-filter columns#23420
Draft
zhuqi-lucas wants to merge 17 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a plan-time heuristic to ParquetSource::try_pushdown_filters to decline Parquet RowFilter pushdown when the scan’s projection is narrow and mostly covered by filter columns, aiming to avoid known regressions when parquet.pushdown_filters is enabled.
Changes:
- Introduces a hardcoded threshold (
PUSHDOWN_MIN_NON_FILTER_COLS = 3) to gate RowFilter pushdown based on projected columns not referenced by filters. - Collects referenced filter columns and projected columns (via
collect_columns) to compute the “non-filter projected” column count before deciding whether to push down.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
@alamb The result looks good also for pushdown benchmark as expected for this PR: Comparing HEAD and qizhu_parquet-pushdown-adaptive-gate
--------------------
Benchmark clickbench_pushdown.json
--------------------
┏━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query ┃ HEAD ┃ qizhu_parquet-pushdown-adaptive-gate ┃ Change ┃
┡━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0 │ 1.24 / 4.10 ±5.59 / 15.28 ms │ 1.22 / 4.01 ±5.45 / 14.91 ms │ no change │
│ QQuery 1 │ 12.96 / 13.15 ±0.12 / 13.30 ms │ 13.57 / 13.86 ±0.28 / 14.39 ms │ 1.05x slower │
│ QQuery 2 │ 36.76 / 37.45 ±0.62 / 38.24 ms │ 36.40 / 36.78 ±0.34 / 37.41 ms │ no change │
│ QQuery 3 │ 31.03 / 31.63 ±0.82 / 33.25 ms │ 31.04 / 31.19 ±0.12 / 31.38 ms │ no change │
│ QQuery 4 │ 227.12 / 230.25 ±2.21 / 233.66 ms │ 226.06 / 227.38 ±0.86 / 228.35 ms │ no change │
│ QQuery 5 │ 274.19 / 277.03 ±2.85 / 282.25 ms │ 273.51 / 278.30 ±3.38 / 283.52 ms │ no change │
│ QQuery 6 │ 1.27 / 1.43 ±0.22 / 1.86 ms │ 1.26 / 1.40 ±0.21 / 1.82 ms │ no change │
│ QQuery 7 │ 16.54 / 16.77 ±0.19 / 16.99 ms │ 15.08 / 15.15 ±0.05 / 15.24 ms │ +1.11x faster │
│ QQuery 8 │ 324.26 / 329.46 ±4.10 / 334.40 ms │ 323.61 / 326.84 ±2.56 / 330.68 ms │ no change │
│ QQuery 9 │ 455.48 / 469.16 ±8.12 / 476.19 ms │ 460.07 / 471.66 ±6.88 / 479.01 ms │ no change │
│ QQuery 10 │ 95.23 / 96.87 ±0.94 / 97.86 ms │ 69.59 / 71.95 ±1.40 / 73.77 ms │ +1.35x faster │
│ QQuery 11 │ 106.87 / 107.79 ±0.95 / 109.41 ms │ 83.37 / 87.98 ±7.69 / 103.25 ms │ +1.23x faster │
│ QQuery 12 │ 305.74 / 309.05 ±2.69 / 313.44 ms │ 268.82 / 272.19 ±2.77 / 277.13 ms │ +1.14x faster │
│ QQuery 13 │ 426.27 / 436.14 ±9.16 / 450.87 ms │ 370.07 / 375.79 ±5.10 / 383.77 ms │ +1.16x faster │
│ QQuery 14 │ 317.97 / 323.55 ±7.98 / 339.25 ms │ 286.21 / 289.30 ±4.51 / 298.04 ms │ +1.12x faster │
│ QQuery 15 │ 275.93 / 284.48 ±7.07 / 292.21 ms │ 274.85 / 287.42 ±11.31 / 305.63 ms │ no change │
│ QQuery 16 │ 622.58 / 633.26 ±9.22 / 649.55 ms │ 614.49 / 623.14 ±7.08 / 632.86 ms │ no change │
│ QQuery 17 │ 634.21 / 637.48 ±2.55 / 641.26 ms │ 623.65 / 634.04 ±9.82 / 646.87 ms │ no change │
│ QQuery 18 │ 1269.87 / 1302.81 ±24.32 / 1345.00 ms │ 1264.80 / 1271.50 ±4.83 / 1278.65 ms │ no change │
│ QQuery 19 │ 29.25 / 29.59 ±0.21 / 29.76 ms │ 28.39 / 36.98 ±15.81 / 68.53 ms │ 1.25x slower │
│ QQuery 20 │ 517.33 / 526.75 ±11.31 / 547.09 ms │ 519.01 / 525.65 ±7.19 / 538.98 ms │ no change │
│ QQuery 21 │ 567.02 / 578.69 ±6.88 / 586.50 ms │ 521.39 / 526.46 ±4.22 / 532.15 ms │ +1.10x faster │
│ QQuery 22 │ 927.00 / 937.75 ±10.48 / 956.68 ms │ 990.15 / 993.76 ±4.16 / 1001.68 ms │ 1.06x slower │
│ QQuery 23 │ 116.66 / 125.46 ±5.87 / 133.61 ms │ 116.37 / 124.25 ±5.85 / 132.68 ms │ no change │
│ QQuery 24 │ 37.83 / 39.80 ±3.22 / 46.20 ms │ 41.28 / 42.28 ±1.88 / 46.05 ms │ 1.06x slower │
│ QQuery 25 │ 144.81 / 149.31 ±3.42 / 153.64 ms │ 112.15 / 119.12 ±7.41 / 132.55 ms │ +1.25x faster │
│ QQuery 26 │ 49.92 / 50.96 ±0.86 / 52.31 ms │ 42.00 / 42.70 ±0.37 / 43.04 ms │ +1.19x faster │
│ QQuery 27 │ 711.11 / 719.52 ±9.57 / 737.84 ms │ 673.75 / 683.93 ±6.63 / 692.00 ms │ no change │
│ QQuery 28 │ 3062.78 / 3091.39 ±19.74 / 3119.93 ms │ 3067.46 / 3077.46 ±7.56 / 3085.45 ms │ no change │
│ QQuery 29 │ 41.17 / 45.16 ±7.39 / 59.93 ms │ 41.71 / 55.51 ±9.80 / 71.56 ms │ 1.23x slower │
│ QQuery 30 │ 312.51 / 317.83 ±4.76 / 326.73 ms │ 307.46 / 314.68 ±4.54 / 321.29 ms │ no change │
│ QQuery 31 │ 293.30 / 301.88 ±6.24 / 310.74 ms │ 295.00 / 306.48 ±11.50 / 328.33 ms │ no change │
│ QQuery 32 │ 926.19 / 968.21 ±25.26 / 1002.64 ms │ 950.24 / 994.38 ±23.11 / 1017.51 ms │ no change │
│ QQuery 33 │ 1451.44 / 1489.42 ±24.90 / 1529.12 ms │ 1474.44 / 1486.27 ±12.10 / 1503.04 ms │ no change │
│ QQuery 34 │ 1486.33 / 1497.38 ±6.12 / 1505.11 ms │ 1493.04 / 1534.83 ±38.86 / 1585.35 ms │ no change │
│ QQuery 35 │ 281.95 / 303.19 ±24.93 / 348.47 ms │ 281.46 / 305.21 ±16.40 / 324.06 ms │ no change │
│ QQuery 36 │ 67.03 / 73.48 ±4.55 / 80.29 ms │ 67.22 / 72.37 ±4.84 / 80.72 ms │ no change │
│ QQuery 37 │ 36.59 / 40.92 ±4.82 / 48.33 ms │ 35.86 / 41.06 ±4.50 / 47.14 ms │ no change │
│ QQuery 38 │ 36.44 / 42.07 ±4.96 / 49.73 ms │ 41.81 / 43.46 ±0.90 / 44.56 ms │ no change │
│ QQuery 39 │ 141.26 / 143.72 ±2.13 / 147.46 ms │ 126.12 / 142.17 ±9.14 / 154.61 ms │ no change │
│ QQuery 40 │ 18.39 / 21.19 ±4.94 / 31.06 ms │ 14.23 / 16.32 ±2.55 / 21.27 ms │ +1.30x faster │
│ QQuery 41 │ 17.21 / 21.75 ±5.29 / 30.45 ms │ 14.19 / 14.49 ±0.22 / 14.82 ms │ +1.50x faster │
│ QQuery 42 │ 14.71 / 14.98 ±0.21 / 15.23 ms │ 13.56 / 13.77 ±0.14 / 13.99 ms │ +1.09x faster │
└───────────┴───────────────────────────────────────┴───────────────────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary ┃ ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD) │ 17072.27ms │
│ Total Time (qizhu_parquet-pushdown-adaptive-gate) │ 16833.48ms │
│ Average Time (HEAD) │ 397.03ms │
│ Average Time (qizhu_parquet-pushdown-adaptive-gate) │ 391.48ms │
│ Queries Faster │ 12 │
│ Queries Slower │ 5 │
│ Queries with No Change │ 26 │
│ Queries with Failure │ 0 │
└─────────────────────────────────────────────────────┴────────────┘ |
alamb
reviewed
Jul 9, 2026
alamb
left a comment
alamb
left a comment
Contributor
There was a problem hiding this comment.
This is great. I think we are very close @zhuqi-lucas
I think we need to do the following :
- To make this a config setting that can be disabled (so that people who want to force filter pushdown on (to keep the current behavior) can do so
- Add a note to the upgrade guide explaining how to get the original behavior
- Add some metric so it is clear from the profiling that this heuristic has happened
I would also like to do some profiling of the query that appears consistently get slower
│ QQuery 29 │ 41.17 / 45.16 ±7.39 / 59.93 ms │ 41.71 / 55.51 ±9.80 / 71.56 ms │ 1.23x slower │
Contributor
|
I will try and work on some of these items |
zhuqi-lucas
added a commit
to zhuqi-lucas/arrow-datafusion
that referenced
this pull request
Jul 10, 2026
… marker Per @alamb's review on apache#23420: 1. Config opt-out: new `datafusion.execution.parquet.pushdown_filter_narrow_projection_gate` (default true). Set to false to restore the pre-existing unconditional pushdown behavior. Proto + serde support included so the option round-trips through the physical plan. 2. Plan-display marker: when the gate declines pushdown for a scan, the ParquetSource fmt_extra output includes `pushdown_declined=narrow_projection` so profiling / EXPLAIN ANALYZE can see the heuristic fired without needing to correlate config against per-query symptoms. 3. Upgrade guide: docs/source/library-user-guide/upgrading/55.0.0.md documents the new behavior and how to opt out. Kept the heuristic itself unchanged (single hardcoded PUSHDOWN_MIN_NON_FILTER_COLS = 3 with no tuning knob) per @alamb's 'complexity budget' note.
github-actions
Bot
added
documentation
Contributor
Author
Thanks @alamb , addressed all three points in 1ef3990:
Kept the heuristic itself untouched (single hardcoded PUSHDOWN_MIN_NON_FILTER_COLS = 3, no tuning knob, no byte-weighting) per your complexity-budget note. Q29 is noise, above triggered another run, and it does not have filter. │ QQuery 29 │ 41.48 / 50.75 ±17.17 / 85.06 ms │ 41.41 / 53.52 ±15.73 / 81.05 ms │ 1.05x slower │ |
|
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 |
zhuqi-lucas
added a commit
to zhuqi-lucas/arrow-datafusion
that referenced
this pull request
Jul 10, 2026
Per Copilot's review on apache#23420 line 887: TableSchema layout is [file, partition, virtual], and only file columns are actually decoded from parquet. Partition and virtual columns don't incur decode cost, so counting them as 'non-filter projected cols' can keep pushdown enabled for partitioned tables whose *file* projection is actually narrow — reintroducing the very regressions this heuristic protects against. Now both the filter-column set and the non-filter projected count restrict to indices `< file_schema().fields().len()`.
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
Author
|
The |
Contributor
Author
|
run benchmarks |
Contributor
Author
|
run benchmark clickbench_partitioned env:
SIMULATE_LATENCY: "true" |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing qizhu/parquet-pushdown-adaptive-gate (cd95750) to 95398f0 (merge-base) diff using: clickbench_partitioned File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing qizhu/parquet-pushdown-adaptive-gate (cd95750) to 95398f0 (merge-base) diff using: clickbench_partitioned File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing qizhu/parquet-pushdown-adaptive-gate (cd95750) to 95398f0 (merge-base) diff using: tpch File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing qizhu/parquet-pushdown-adaptive-gate (cd95750) to 95398f0 (merge-base) diff using: tpcds File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
Author
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing qizhu/parquet-pushdown-adaptive-gate (a318fb3) to 761f7f6 (merge-base) diff using: tpch File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing qizhu/parquet-pushdown-adaptive-gate (a318fb3) to 761f7f6 (merge-base) diff using: tpcds File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing qizhu/parquet-pushdown-adaptive-gate (a318fb3) to 761f7f6 (merge-base) diff using: clickbench_partitioned File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
zhuqi-lucas
force-pushed
the
qizhu/parquet-pushdown-adaptive-gate
branch
from
a318fb3 to
f4ec013
Compare
Contributor
Author
|
Reverted the PR to only included the flip default to true commit, so better for us to profile more details. |
zhuqi-lucas
force-pushed
the
qizhu/parquet-pushdown-adaptive-gate
branch
from
f4ec013 to
e6e0072
Compare
Contributor
|
@zhuqi-lucas -- what is the status of this PR? I sort of lost track with all the changes. Is there anything I can do to help? |
Contributor
Author
Thanks @alamb. Marked as draft — hit a few regressions I couldn't fully explain in this PR's scope:
Tried something to improve it, but i can't make it worked well, so i force-pushed back to the minimal "flip default + |
Contributor
Author
|
Update on the TPCH regression from my previous comment (@alamb): Opened #23532 — caches the remapped expression inside TPCH SF1 (10 iterations,
Q17 reaches parity with This doesn't resolve the whole TPCH regression from the earlier comment. Remaining gap between #23532 (644 ms) and ClickBench simulate-latency and tpcds numbers I'll re-run against #23532 as follow-up. |
Contributor
The fundamental issue is that (1) evaluating partition routing hashes is expensive and (2) is sometimes a big win if the join is selective but is the join is a 90% match you end up just paying the hashing price twice for no benefit, especially if the late materialization benefit is small as well. We can't know this statically, so yes I think the only way to work around it is to dynamically drop dynamic filters that are not earning their keep.
I think that would be very nice. It is both a good chunk of the code but also resolves some theoretical issues that mean we could return wrong results in cases where |
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
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?
filter_pushdown) by default #3463Part of #3463 (Enable
parquet.pushdown_filtersby default) — this is the first step of the two-step split @alamb suggested on #23369: land the heuristic on its own now; flip the default in a follow-up.Rationale for this change
RowFilter has a fixed per-row machinery overhead that only pays for itself when the wide-column decode it lets us skip is meaningful. When the projection is narrow and the filter columns already cover most of it (typical for
GROUP BY col-style queries such as ClickBench Q10/Q11/Q40), the overhead dominates and pushdown regresses.On ClickBench with
pushdown_filters=true, the naive path shows 20 slower / 6 faster / 17 no change vs HEAD. Adding this gate on top gave 4 slower / 5 faster / 34 no change (see #23369 CI comparison), with the remaining 4 "slower" queries all inside the CI noise floor. Q23 keeps its ~21× speedup.What changes are included in this PR?
Single-file change in
ParquetSource::try_pushdown_filters: decline pushdown when the projection contains fewer than 3 columns not referenced by the filter. When declined,pushdown_filtersis treated as disabled for that scan — the filter stays above the scan in aFilterExec(correctness preserved) and the predicate is still injected intoParquetSourcefor stats / bloom / page-index pruning.Kept intentionally simple:
PUSHDOWN_MIN_NON_FILTER_COLS = 3.Are these changes tested?
By existing tests: this change only affects scans where the user has opted into
pushdown_filters=true(config orParquetSource::pushdown_filters()), and only in the direction of declining pushdown (falling back to the pre-existingFilterExecpath). The defaultpushdown_filters=falsebehavior is unchanged, so the default test matrix already covers the fallback path.The ClickBench comparison on #23369 provides the end-to-end validation.
Are there any user-facing changes?
For users who explicitly enable
pushdown_filters=true: their queries with narrow projections (fewer than 3 non-filter columns) will no longer push filters into the Parquet scan. This eliminates the Q10-style regressions those users historically hit. Users on the default (pushdown_filters=false) see zero change.Follow-up
pushdown_filtersdefault totrueonce we're confident this heuristic holds up.cc @alamb