zhuqi-lucas commented on code in PR #23420:
URL: https://github.com/apache/datafusion/pull/23420#discussion_r3557106421
##########
datafusion/datasource-parquet/src/source.rs:
##########
@@ -827,7 +828,43 @@ impl FileSource for ParquetSource {
// because even if scan pushdown is disabled we can still use the
filters for stats pruning.
let config_pushdown_enabled =
config.execution.parquet.pushdown_filters;
let table_pushdown_enabled = self.pushdown_filters();
- let pushdown_filters = table_pushdown_enabled ||
config_pushdown_enabled;
+ let mut pushdown_filters = table_pushdown_enabled ||
config_pushdown_enabled;
+
+ // Simple heuristic (issue #3463, discussion on PR #23369):
+ // 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. Decline pushdown for those scans; keep it
+ // for wider projections where the fast-path pays.
+ //
+ // Kept intentionally simple: a plan-time column-count check with
+ // no tuning knob. A future adaptive-placement pass (#22883) can
+ // supersede this with a runtime cost model.
+ //
+ // When declined: 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.
+ const PUSHDOWN_MIN_NON_FILTER_COLS: usize = 3;
+ if pushdown_filters {
+ let filter_col_indices: std::collections::HashSet<usize> = filters
+ .iter()
+ .flat_map(|f| collect_columns(f).into_iter().map(|c|
c.index()))
+ .collect();
+ let non_filter_projected = self
+ .projection
+ .as_ref()
+ .iter()
+ .flat_map(|pe| collect_columns(&pe.expr))
+ .map(|c| c.index())
+ .filter(|idx| !filter_col_indices.contains(idx))
+ .collect::<std::collections::HashSet<_>>()
+ .len();
Review Comment:
Good catch — done in 25208ebe. Both the filter-column set and the non-filter
projected count now restrict to indices `<
self.table_schema.file_schema().fields().len()`, so partition and virtual
columns are excluded from the "wide-decode saving" calculation.
--
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]
