zhuqi-lucas opened a new pull request, #23420:
URL: https://github.com/apache/datafusion/pull/23420

   ## Which issue does this PR close?
   
   Part of [#3463](https://github.com/apache/datafusion/issues/3463) (Enable 
`parquet.pushdown_filters` by default) — this is the first step of the two-step 
split @alamb suggested on 
[#23369](https://github.com/apache/datafusion/pull/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](https://github.com/apache/datafusion/pull/23369)), 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_filters` is treated as disabled for that 
scan — the filter stays above the scan in a `FilterExec` (correctness 
preserved) and the predicate is still injected into `ParquetSource` for stats / 
bloom / page-index pruning.
   
   Kept intentionally simple:
   
   - No tuning knob — a hardcoded `PUSHDOWN_MIN_NON_FILTER_COLS = 3`.
   - No byte-weighted / data-type-aware cost model.
   - Complexity budget reserved for the runtime adaptive-placement work in 
[#22883](https://github.com/apache/datafusion/issues/22883), which will 
supersede this heuristic when it lands.
   
   ## Are these changes tested?
   
   By existing tests: this change only affects scans where the user has *opted 
into* `pushdown_filters=true` (config or `ParquetSource::pushdown_filters()`), 
and only in the direction of *declining* pushdown (falling back to the 
pre-existing `FilterExec` path). The default `pushdown_filters=false` behavior 
is unchanged, so the default test matrix already covers the fallback path.
   
   The ClickBench comparison on 
[#23369](https://github.com/apache/datafusion/pull/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
   
   - Extended benchmarks across other workloads (per @alamb).
   - Follow-up PR to flip `pushdown_filters` default to `true` once we're 
confident this heuristic holds up.
   - Longer term: [#22883](https://github.com/apache/datafusion/issues/22883) 
runtime adaptive placement.
   
   cc @alamb


-- 
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]

Reply via email to