tohuya6 opened a new pull request, #23691:
URL: https://github.com/apache/datafusion/pull/23691
## Which issue does this PR close?
Closes #23655.
## Rationale for this change
`SELECT f(c)['a'], f(c)['b']`, where `f` is a struct-returning UDF,
evaluates `f(c)` twice in the physical `ProjectionExec` — once per `get_field`
— although the two calls are identical.
`CommonSubexprEliminate` already hoists the shared `f(c)` into a
`__common_expr_N` column so it runs once. `PushDownLeafProjections` then undoes
that, one pass at a time:
- After CSE, each field access reads its base from the hoist column, so
`get_field`'s placement flips to `MoveTowardsLeafNodes` and the pass extracts
it.
- Merging the extraction into the hoist projection inlines that column's
definition back into every access (via `build_projection_replace_map`),
re-duplicating the `KeepInPlace` UDF.
- `optimize_projections` drops the now-unused hoist column, and the two
passes oscillate pass after pass — settling on a double evaluation.
This is the instability the `is_ignored` note in `common_subexpr_eliminate`
already warns about.
## What changes are included in this PR?
`split_and_push_projection` now bails out — leaving the projection untouched
— when merging the extractions into the input projection would inline a
`KeepInPlace` definition that more than one extraction pair references. This
mirrors the existing `optimize_projections` merge guard (#8296): a compute-once
expression referenced more than once is kept in place instead of duplicated.
With the guard, CSE's hoist survives, `optimize_projections` keeps it, and
the pipeline converges to a single evaluation:
Before:
```text
Projection: get_field(f(c), 'a'), get_field(f(c), 'b') -- f(c) twice
TableScan
```
After:
```text
Projection: get_field(__common_expr_1, 'a'), get_field(__common_expr_1, 'b')
Projection: f(c) AS __common_expr_1 -- f(c) once
TableScan
```
## Are these changes tested?
Yes.
- `test_struct_returning_udf_evaluated_once` (in
`extract_leaf_expressions.rs`) runs the four interacting rules — CSE →
extraction → pushdown → projection pruning — and asserts no field access wraps
a fresh UDF call, plus an `insta` snapshot of the single-eval plan. It runs
with the default pass budget, so any residual oscillation would surface as a
double evaluation.
- `get_field_like` — a small test UDF added to `test/udfs.rs` — reproduces
`GetFieldFunc`'s conditional placement (`MoveTowardsLeafNodes` only when the
base is pushable and the keys are literals). This lets the optimizer crate
model struct-field access without depending on `datafusion-functions`, which it
intentionally does not.
`cargo fmt --all` and `cargo clippy --all-targets --all-features -- -D
warnings` are clean.
## Are there any user-facing changes?
No API changes. Affected plans evaluate a struct-returning UDF once instead
of once per field access; query results are unchanged. The saving scales with
the number of distinct field accesses on the same UDF.
## Follow-ups
- An end-to-end `.slt` test would need a struct-returning UDF registered in
the sqllogictest harness: the built-in `named_struct` / `struct` constructors
are folded into their fields by the field-over-struct-literal rewrite before
this interaction can occur, so only an opaque (UDF) struct source reproduces
it. Covered at the optimizer layer here instead.
--
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]
