adriangb opened a new issue, #25447:
URL: https://github.com/apache/datafusion/issues/25447
### Describe the bug
Leaf extraction puts a cheap expression into its own projection so that the
plan computes it one time. When two plan nodes need the same expression, the
merge step writes the expression into that projection two times, under two
different `__datafusion_extracted_N` names. The duplicate survives into the
physical plan, so the expression runs two times for every row.
`datafusion.optimizer.enable_leaf_expression_pushdown` is `true` by default.
### To Reproduce
Run these statements in `datafusion-cli` at `main` (`3a647e49dd`).
```sql
set datafusion.explain.format = 'indent';
create table t(v int, s struct<a int, b int>) as values (1, {a: 10, b: 1}),
(2, {a: 20, b: 2});
explain select s['a'] from (select s, v from t where v > 0) where s['a'] > 0;
```
```text
logical_plan
Projection: __datafusion_extracted_2 AS t.s[a]
Filter: __datafusion_extracted_1 > Int32(0)
Projection: __datafusion_extracted_1, __datafusion_extracted_2
Filter: t.v > Int32(0)
Projection: get_field(t.s, Utf8("a")) AS __datafusion_extracted_1,
get_field(t.s, Utf8("a")) AS __datafusion_extracted_2, t.v
TableScan: t projection=[v, s]
physical_plan
ProjectionExec: expr=[__datafusion_extracted_2@0 as t.s[a]]
FilterExec: __datafusion_extracted_1@0 > 0 AND v@2 > 0,
projection=[__datafusion_extracted_2@1]
ProjectionExec: expr=[get_field(s@1, a) as __datafusion_extracted_1,
get_field(s@1, a) as __datafusion_extracted_2, v@0 as v]
DataSourceExec: partitions=1, partition_sizes=[1]
```
`get_field(s, a)` is in the bottom `ProjectionExec` two times.
### Expected behavior
The bottom projection computes `get_field(t.s, "a")` one time. The filter
above it and the projection above that both read the same column.
### Additional context
The merge happens in the extracted expression loop of
`build_extraction_projection_impl`, in
`datafusion/optimizer/src/extract_leaf_expressions.rs`:
```rust
if let Some(existing_alias) = existing_extractions.get(resolved_inner) {
// Same expression already extracted under a different alias —
// add the expression with the new alias so both names are
// available in the output. We can't reference the existing alias
// as a column within the same projection, so we duplicate the
// computation.
if existing_alias != alias {
proj_exprs.push(resolved);
}
} else {
proj_exprs.push(resolved);
}
```
Two paths reach the duplicate.
**Path 1, the alias differs.** The two passes give the same expression two
names. The loop finds the existing alias, sees a different name, and writes a
second copy. The statement above takes this path. A projection cannot refer to
its own output column, so the copy cannot simply be replaced by a column
reference. The consumers above have to be rewritten onto the alias that is
already there.
**Path 2, the lookup misses.** The loop matches on expression equality after
`replace_cols_by_name`, and that helper matches on `Column::flat_name()`. A
column that arrives unqualified while the projection output names it `t.s` is
not rewritten, so `get_field(s, "a")` and `get_field(t.s, "a")` do not compare
equal and the lookup finds nothing. This is the same mismatch that
https://github.com/apache/datafusion/pull/25412 fixes one loop further down,
for pass-through columns.
Path 2 is masked on `main` today, because the shapes that produce an
unqualified spelling fail first with the error that PR 25412 fixes. I built the
head of that PR and ran this statement on it:
```sql
create table t(v int, s struct<a int, b int>, env varchar) as values (1, {a:
10, b: 1}, 'prod'), (2, {a: 20, b: 2}, 'dev');
explain select env, sum(s['a'])
from (select v, s, env from t where s['a'] > 0
union all
select v, s, env from t where 1 = 2)
group by env;
```
The merged projection holds the two spellings side by side:
```text
Projection: get_field(t.s, Utf8("a")) AS __datafusion_extracted_2, t.env,
get_field(s, Utf8("a")) AS __datafusion_extracted_1
```
### Code pointers
- `datafusion/optimizer/src/extract_leaf_expressions.rs`,
`build_extraction_projection_impl`, the `extracted_exprs` loop.
- `datafusion/optimizer/src/extract_leaf_expressions.rs`,
`build_projection_replace_map`. The map is keyed on `Column::flat_name()`.
- `datafusion/optimizer/src/push_down_filter.rs`, `replace_cols_by_name`.
The lookup is an exact `flat_name()` match.
### Related
- https://github.com/apache/datafusion/pull/25412 fixes the same name space
mismatch in the pass-through loop of the same function.
- https://github.com/apache/datafusion/issues/25329 is a different duplicate
evaluation, caused by inlining a `KeepInPlace` definition.
- `would_duplicate_costly_exprs` in
`datafusion/datasource/src/file_scan_config/mod.rs` already refuses to merge a
projection into the scan when an expression is referenced more than one time.
The logical rule creates the duplicate before that check sees the plan.
Tracked in the leaf-pushdown EPIC.
--
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]
