fordN opened a new pull request, #23459:
URL: https://github.com/apache/datafusion/pull/23459
## Which issue does this PR close?
- Closes #23425.
Follow-up to #23395 (the volatile correctness fix, now merged), extending
the same projection-pushdown guard to the performance case.
## Rationale for this change
The logical CSE pass extracts a repeated scalar-function call (e.g.
`power(a, 2)`) into a single intermediate projection referenced by column, so
it is evaluated once per row. For file sources that can absorb computed
projections (e.g. Parquet), the physical projection-pushdown rule then merges
that projection into the scan's `DataSourceExec`, re-inlining the expression at
every reference site and re-evaluating it N times per row — undoing the
deduplication.
This is the performance sibling of #23220 (which fixed the *correctness*
case for volatile expressions). It reproduces on file scans (Parquet/CSV), not
in-memory tables.
**EXPLAIN, before** — `power(a, 2)` evaluated 3× per row:
```
DataSourceExec: projection=[power(a,2)+b as x, power(a,2)-b as y,
power(a,2)*c as z]
```
**EXPLAIN, after** — `power(a, 2)` evaluated once, kept in a
`ProjectionExec`:
```
ProjectionExec: [__common_expr_1+b as x, __common_expr_1-b as y,
__common_expr_1*c as z]
DataSourceExec: projection=[power(a,2) as __common_expr_1, b, c]
```
## What changes are included in this PR?
- Extends the projection-pushdown guard in
`FileScanConfig::try_swapping_with_projection` so it also declines the merge
when it would duplicate a **non-trivial** expression. "Non-trivial" is decided
by the existing expression-placement signal (`KeepInPlace`) — the same signal
`try_collapse_projection_chain` uses — covering arithmetic, casts, and most
scalar functions. Cheap leaf-pushable expressions (columns, `get_field`,
`input_file_name`) still merge, so struct-field pushdown is unaffected. It
reuses the volatile guard's multiplicity-aware reference counting, so an
expression is blocked only when referenced more than once.
- Adds the `cse_projection_pushdown` benchmark used to measure the change.
## Are these changes tested?
Yes:
- Unit tests in `file_scan_config.rs`: a non-trivial expr (arithmetic /
scalar function) referenced ≥2 → blocked, once → allowed; a leaf-pushable
scalar function (`get_field`) → allowed; volatile referenced ≥2 → blocked, once
→ allowed.
- `datafusion-sqllogictest` suite passes. One existing plan in `window.slt`
improves: a repeated `c2 >= 2` comparison over a CSV scan is now computed once
instead of being inlined twice into the `DataSourceExec`.
- Benchmark A/B on `cse_projection_pushdown` (sample-size 50, 5s), with the
`no_repeated_exprs` control confirming no change on unaffected queries:
| Benchmark | Change |
|---|---|
| `repeated_power` (`power(a,2)` ×3) | −40% |
| `repeated_nested_fn` (`ln(abs(a))` ×3) | −38% |
| `repeated_sqrt` (`sqrt(a)` ×3) | −37% |
| `mixed_repeated_unique` | −35% |
| `repeated_cheap_abs` (`abs(a)` ×3) | within noise |
| `no_repeated_exprs` (control) | no change |
The gain scales with expression cost. For a single-instruction function like
`abs`, caching the value in a `ProjectionExec` versus recomputing it is a wash
(run-to-run noise), since the extra plan node roughly offsets the recomputation
it saves.
Reproduce with:
```
cargo bench -p datafusion --bench cse_projection_pushdown --features parquet
```
## Are there any user-facing changes?
No API changes. Queries that repeat an expensive expression over a file scan
run faster; their physical plans retain a `ProjectionExec` above the scan (the
expression evaluated once) instead of inlining it into the `DataSourceExec`
projection.
--
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]
