You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/14 20:05:38 UTC
(datafusion) branch gh-readonly-queue/main/pr-25412-6079c8b67bbd88c22fc7a5cb153e130d4e9532bb created (now 25e0ab2b26)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-25412-6079c8b67bbd88c22fc7a5cb153e130d4e9532bb
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 25e0ab2b26 fix: resolve pass-through columns against the input when merging an extraction projection (#25412)
This branch includes the following new commits:
new 25e0ab2b26 fix: resolve pass-through columns against the input when merging an extraction projection (#25412)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: resolve pass-through columns against the input when merging an extraction projection (#25412)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25412-6079c8b67bbd88c22fc7a5cb153e130d4e9532bb
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 25e0ab2b261e7cb2dc186e8c983143bd4a4bbb1a
Author: Adrian Garcia Badaracco <17...@users.noreply.github.com>
AuthorDate: Wed Oct 14 20:05:14 2026 +0000
fix: resolve pass-through columns against the input when merging an extraction projection (#25412)
## Which issue does this PR close?
None filed.
## Rationale for this change
This query fails to plan on `main`:
```sql
create table t(v int, s struct<a int>, env varchar) as values (1, {a: 10}, 'prod'), (2, {a: 20}, 'dev');
with samples as (
select v, s, env from t
),
expanded as (
select v, s, env from samples
union all
select v, s, env from samples where 1 = 2
)
select env, sum(s['a']) from expanded group by env;
```
```
Optimizer rule 'push_down_leaf_projections' failed
caused by
Schema error: Schema contains qualified field name samples.env and unqualified
field name env which would be ambiguous
```
`enable_leaf_expression_pushdown` is on by default, so this is a plain
planning
failure for a valid statement. A `union all` whose one side is provably
empty is
what produces the shape, and query generators emit that: a dashboard
panel adds a
second branch behind a comparison of two constants and the panel stops
working.
## What changes are included in this PR?
`build_extraction_projection_impl` merges an extraction projection into
the
projection below it, and then adds the pass-through columns the merged
projection
does not already carry. It compared the columns it was about to add
against the
projection's own expressions without putting the two in the same space.
A projection can list bare column names over a qualified input. Removing
the empty
side of the union leaves exactly that. The comparison then misses, the
column is
added a second time under its bare name, and `Projection::try_new`
rejects a schema
that holds `samples.env` and a bare `env` together.
This PR resolves both sides against the input schema before the
comparison, and
pushes the column under the name the input gives it. A needed column
that is not in
the input, or that is ambiguous there, is skipped. Before this change
such a column
was added and `Projection::try_new` failed at the merge.
A same-name alias such as `t.c AS c` counts as a pass-through on the
existing side too, the same as a bare `t.c`.
## Are these changes tested?
Yes. The statement above is added to `struct.slt`, with a result check
and an
`EXPLAIN` plan pin. It fails on `main` with the error above and passes
with this change.
Three unit tests in `extract_leaf_expressions.rs` cover the merge
directly. All three fail on `main`:
- `test_merge_bare_column_into_qualified_projection`: a filter names `a`
over a projection that outputs `test.a`.
- `test_merge_into_projection_with_same_name_alias`: the projection
spells the pass-through as `test.a AS a`.
- `test_merge_appends_qualified_column_for_bare_request`: the projection
keeps only `test.a` and the merge needs a bare `b`. The appended column
must be `test.b`. On `main` it is a bare `b`.
`cargo test --workspace --exclude datafusion-sqllogictest` passes: 126
suites,
12479 tests, 0 failures. The sqllogictest suite passes too: 520 files, 0
failures. `cargo clippy -p datafusion-optimizer --all-targets` and
`cargo fmt --all -- --check` are clean.
## Are there any user-facing changes?
A statement of this shape plans instead of failing. No API change.
## Note on overlapping work
https://github.com/apache/datafusion/pull/25388 (draft) edits the same
arm of
`build_extraction_projection_impl`, for a different problem: duplicated
evaluation of `KeepInPlace` expressions,
https://github.com/apache/datafusion/issues/25329.
It does not touch the qualifier comparison this PR changes, so the two
are
independent in behaviour, but whichever lands second will need a small
rebase at
the tail of the `columns_needed` loop.
https://github.com/apache/datafusion/pull/25282 also appends to the end
of
`struct.slt`, as this PR does. Keeping both blocks is the whole
resolution.
Part of the leaf-pushdown EPIC:
https://github.com/apache/datafusion/issues/25459
---------
Co-authored-by: Claude Opus 5.5 <no...@anthropic.com>
---
.../optimizer/src/extract_leaf_expressions.rs | 147 +++++++++++++++++++--
datafusion/sqllogictest/test_files/struct.slt | 51 +++++++
2 files changed, 186 insertions(+), 12 deletions(-)
diff --git a/datafusion/optimizer/src/extract_leaf_expressions.rs b/datafusion/optimizer/src/extract_leaf_expressions.rs
index aa3cef881f..999c396745 100644
--- a/datafusion/optimizer/src/extract_leaf_expressions.rs
+++ b/datafusion/optimizer/src/extract_leaf_expressions.rs
@@ -672,6 +672,20 @@ impl<'a> LeafExpressionExtractor<'a> {
}
}
+/// Resolves `col` against `schema` and returns it with the qualifier that
+/// `schema` gives the field. Returns `None` if `schema` does not hold `col`,
+/// or if the name is ambiguous in `schema`.
+///
+/// Two spellings of the same input column (a bare `c` and a qualified `t.c`)
+/// resolve to the same `Column`, so they compare equal. A column pushed into
+/// a projection this way uses the name that the input gives it.
+fn resolve_against(schema: &DFSchema, col: &Column) -> Option<Column> {
+ schema
+ .qualified_field_from_column(col)
+ .ok()
+ .map(Column::from)
+}
+
/// Build an extraction projection above the target node (shared by both passes).
///
/// If the target is an existing projection, merges into it. This requires
@@ -737,29 +751,31 @@ fn build_extraction_projection_impl(
// than target_schema (the projection's output) because columns produced
// by alias expressions (e.g., CSE's __common_expr_N) exist in the output but
// not the input, and cannot be added as pass-through Column references.
+ //
+ // Resolve the pass-through columns of `existing` (including same-name
+ // aliases such as `t.c AS c`) and the columns in `columns_needed`
+ // against the input schema before comparing them. Otherwise a bare `c`
+ // and a qualified `t.c` do not match, and the merged projection holds
+ // both, which `Projection::try_new` rejects as ambiguous.
+ let input_schema = existing.input.schema();
let existing_cols: IndexSet<Column> = existing
.expr
.iter()
- .filter_map(|e| {
- if let Expr::Column(c) = e {
- Some(c.clone())
- } else {
- None
- }
- })
+ .filter_map(|e| resolve_against(input_schema, passthrough_column(e)?))
.collect();
- let input_schema = existing.input.schema();
for col in columns_needed {
let col_expr = Expr::Column(col.clone());
let resolved = replace_cols_by_name(col_expr, &replace_map)?;
if let Expr::Column(resolved_col) = &resolved
- && !existing_cols.contains(resolved_col)
- && input_schema.has_column(resolved_col)
+ && let Some(input_col) = resolve_against(input_schema, resolved_col)
+ && !existing_cols.contains(&input_col)
{
- proj_exprs.push(Expr::Column(resolved_col.clone()));
+ proj_exprs.push(Expr::Column(input_col));
}
- // If resolved to non-column expr, it's already computed by existing projection
+ // Skip a column that resolves to a non-column expr: the existing
+ // projection already computes it. Also skip a column that is not in
+ // the input or is ambiguous there: no single spelling is correct.
}
Projection::try_new(proj_exprs, Arc::clone(&existing.input))
@@ -2405,6 +2421,113 @@ mod tests {
"#)
}
+ /// A filter can name a column bare (`a`) while the projection below it
+ /// outputs the qualified `test.a`. The optimizer leaves this shape when it
+ /// removes the empty input of a `UNION ALL` (for example
+ /// `... UNION ALL SELECT ... WHERE 1 = 2`). The merge must match the two, and not add a bare `a`
+ /// beside `test.a`, which is an ambiguous schema.
+ #[test]
+ fn test_merge_bare_column_into_qualified_projection() -> Result<()> {
+ let table_scan = test_table_scan()?;
+ let projection = LogicalPlanBuilder::from(table_scan)
+ .project(vec![
+ col("test.a"),
+ col("test.b"),
+ (col("test.c") + lit(1)).alias("d"),
+ ])?
+ .build()?;
+ let predicate =
+ leaf_udf(Expr::Column(Column::new_unqualified("a")), "x").eq(lit(1));
+ let plan = LogicalPlan::Filter(datafusion_expr::Filter::try_new(
+ predicate,
+ Arc::new(projection),
+ )?);
+
+ assert_stages!(plan, @r#"
+ ## Original Plan
+ Filter: leaf_udf(a, Utf8("x")) = Int32(1)
+ Projection: test.a, test.b, test.c + Int32(1) AS d
+ TableScan: test projection=[a, b, c]
+
+ ## After Extraction
+ Projection: test.a, test.b, d
+ Filter: __datafusion_extracted_1 = Int32(1)
+ Projection: test.a, test.b, test.c + Int32(1) AS d, leaf_udf(a, Utf8("x")) AS __datafusion_extracted_1
+ TableScan: test projection=[a, b, c]
+
+ ## After Pushdown
+ (same as after extraction)
+
+ ## Optimized
+ (same as after pushdown)
+ "#)
+ }
+
+ /// When the merge appends a needed column that the projection does not
+ /// keep, it must append the input's qualified column (`test.b`), not the
+ /// bare `b` that the caller asked for.
+ #[test]
+ fn test_merge_appends_qualified_column_for_bare_request() -> Result<()> {
+ let projection = LogicalPlanBuilder::from(test_table_scan()?)
+ .project(vec![col("test.a")])?
+ .build()?;
+ let target_schema = Arc::clone(projection.schema());
+ let columns_needed: IndexSet<Column> =
+ IndexSet::from([Column::new_unqualified("b")]);
+
+ let merged = build_extraction_projection_impl(
+ &[],
+ &columns_needed,
+ &Arc::new(projection),
+ &target_schema,
+ )?;
+
+ assert_eq!(
+ merged.expr,
+ vec![
+ Expr::Column(Column::new(Some("test"), "a")),
+ Expr::Column(Column::new(Some("test"), "b")),
+ ]
+ );
+ Ok(())
+ }
+
+ /// A projection can spell a pass-through column as a same-name alias
+ /// (`test.a AS a`). Merging an extraction into it must treat that alias as
+ /// the pass-through it is, and not add `test.a` beside the `a` it outputs,
+ /// which is an ambiguous schema.
+ #[test]
+ fn test_merge_into_projection_with_same_name_alias() -> Result<()> {
+ let table_scan = test_table_scan()?;
+ let plan = LogicalPlanBuilder::from(table_scan)
+ .project(vec![
+ col("test.a").alias("a"),
+ col("test.b").alias("b"),
+ col("test.c").alias("c"),
+ ])?
+ .filter(leaf_udf(col("a"), "x").eq(lit(1)))?
+ .build()?;
+
+ assert_stages!(plan, @r#"
+ ## Original Plan
+ Filter: leaf_udf(a, Utf8("x")) = Int32(1)
+ Projection: test.a AS a, test.b AS b, test.c AS c
+ TableScan: test projection=[a, b, c]
+
+ ## After Extraction
+ Projection: a, b, c
+ Filter: __datafusion_extracted_1 = Int32(1)
+ Projection: test.a AS a, test.b AS b, test.c AS c, leaf_udf(test.a, Utf8("x")) AS __datafusion_extracted_1
+ TableScan: test projection=[a, b, c]
+
+ ## After Pushdown
+ (same as after extraction)
+
+ ## Optimized
+ (same as after pushdown)
+ "#)
+ }
+
// =========================================================================
// Join extraction tests
// =========================================================================
diff --git a/datafusion/sqllogictest/test_files/struct.slt b/datafusion/sqllogictest/test_files/struct.slt
index aa80ff0253..2eb91b712f 100644
--- a/datafusion/sqllogictest/test_files/struct.slt
+++ b/datafusion/sqllogictest/test_files/struct.slt
@@ -1906,3 +1906,54 @@ from (values (1),(2),(1)) group by 1;
----
{a: 1, z: NULL} 2
{a: 2, z: NULL} 1
+
+# Merging an extraction projection into a projection whose output lost its
+# qualifier. Eliminating the empty side of the union leaves a projection of
+# bare column names over a qualified input, and the merge used to add the
+# pass-through columns under those bare names beside the qualified ones the
+# projection already carried, which is an ambiguous schema.
+statement ok
+create table leaf_merge_source(v int, s struct<a int>, env varchar) as values (1, {a: 10}, 'prod'), (2, {a: 20}, 'dev');
+
+statement ok
+set datafusion.explain.logical_plan_only = true;
+
+query TT
+explain with samples as (
+ select v, s, env from leaf_merge_source
+),
+expanded as (
+ select v, s, env from samples
+ union all
+ select v, s, env from samples where 1 = 2
+)
+select env, sum(s['a']) from expanded group by env order by env;
+----
+logical_plan
+01)Sort: expanded.env ASC NULLS LAST
+02)--Projection: expanded.env, sum(__datafusion_extracted_1) AS sum(expanded.s[a])
+03)----Aggregate: groupBy=[[expanded.env]], aggr=[[sum(CAST(__datafusion_extracted_1 AS Int64))]]
+04)------SubqueryAlias: expanded
+05)--------SubqueryAlias: samples
+06)----------Projection: leaf_merge_source.env, get_field(s, Utf8("a")) AS __datafusion_extracted_1
+07)------------TableScan: leaf_merge_source projection=[s, env]
+
+statement ok
+set datafusion.explain.logical_plan_only = false;
+
+query TI
+with samples as (
+ select v, s, env from leaf_merge_source
+),
+expanded as (
+ select v, s, env from samples
+ union all
+ select v, s, env from samples where 1 = 2
+)
+select env, sum(s['a']) from expanded group by env order by env;
+----
+dev 20
+prod 10
+
+statement ok
+drop table leaf_merge_source;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org