From 0900ca49f4eac70a95db411058ce0828a6339654 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:39:46 -0500 Subject: [PATCH 1/2] fix: keep the recovery projection for a computed same-name column `split_and_push_projection` decided if it needs a recovery projection from the set of unqualified field names of the pushed plan. A name says nothing about the value behind it. A projection such as `(- t.a) AS a, t.s, get_field(t.s, "b") AS __datafusion_extracted_1` keeps every name when the extraction goes below it, but the pushed plan exposes the table column `t.a` where the projection computed `- t.a`. The recovery projection went away and the computed column became its own input column. The recovery projection now also stays when a recovery expression computes a value, that is, when it is not a pass-through of a column. The name comparison stays for the leaked-column case that it was written for. Closes https://github.com/apache/datafusion/issues/25414 Co-Authored-By: Claude Fable 5.1 --- .../optimizer/src/extract_leaf_expressions.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/datafusion/optimizer/src/extract_leaf_expressions.rs b/datafusion/optimizer/src/extract_leaf_expressions.rs index 8a4abfcb48e56..1dcc176092ac8 100644 --- a/datafusion/optimizer/src/extract_leaf_expressions.rs +++ b/datafusion/optimizer/src/extract_leaf_expressions.rs @@ -1111,6 +1111,18 @@ fn split_and_push_projection( // `SubqueryAlias` re-qualification (`sub.__datafusion_extracted_1` vs // `__datafusion_extracted_1`) that a qualified/ordered comparison would // spuriously treat as drift, stacking redundant recovery projections. + // + // A name comparison alone is not sufficient. A name says nothing about the + // *value* behind it. Take the projection + // `(- t.a) AS a, t.s, get_field(t.s, "b") AS __datafusion_extracted_1`. When + // the extraction goes below it, the pushed plan keeps every name, but it + // exposes the table column `t.a` where the projection computed `- t.a`. If + // the recovery projection goes away, the computed column becomes its own + // input column and the query gives wrong results. See + // . + // + // So the recovery projection also stays when a recovery expression computes + // a value, that is, when it is not a pass-through of a column. let base_names: BTreeSet<&str> = base_plan .schema() .fields() @@ -1122,7 +1134,10 @@ fn split_and_push_projection( .iter() .map(|f| f.name().as_str()) .collect(); - let needs_recovery = base_names != original_names; + let computes_a_value = recovery_exprs + .iter() + .any(|expr| passthrough_column(expr).is_none()); + let needs_recovery = base_names != original_names || computes_a_value; // Wrap with recovery projection if the output schema changed if needs_recovery { @@ -3601,4 +3616,5 @@ mod tests { Ok(()) } + } From 2bb9d19ab75615d5b1f7fdde8f7b908800074996 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:40:29 -0500 Subject: [PATCH 2/2] test: add regression tests for a computed same-name column Six SQL shapes go in `struct.slt`: through a Filter, through a Limit, a computed column with a different type, an outer filter on the computed column, a group key, and through a Sort. Five of them gave wrong results or an internal error before the fix. The Sort shape was already correct and guards it. One EXPLAIN shows that the projection that computes `-a` stays in the plan. The unit test builds the plan shape that loses the computed column and runs the two leaf rules alone, in their production order. Co-Authored-By: Claude Fable 5.1 --- .../optimizer/src/extract_leaf_expressions.rs | 38 ++++++++++ datafusion/sqllogictest/test_files/struct.slt | 69 +++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/datafusion/optimizer/src/extract_leaf_expressions.rs b/datafusion/optimizer/src/extract_leaf_expressions.rs index 1dcc176092ac8..f78094a6da261 100644 --- a/datafusion/optimizer/src/extract_leaf_expressions.rs +++ b/datafusion/optimizer/src/extract_leaf_expressions.rs @@ -3617,4 +3617,42 @@ mod tests { Ok(()) } + /// Regression test for . + /// + /// `(- test.id) AS id` computes a new value under the same name as its input + /// column `test.id`. Pushing the extraction below that projection makes + /// `test.id` visible again under the name `id`. The recovery projection must + /// stay, or the computed column is silently replaced by the table column. + /// + /// The two leaf rules run alone here, in their production order. + /// `optimize_projections` merges the two projections into one and hides the + /// shape, and it only runs after both leaf rules. + #[test] + fn test_recovery_kept_for_same_name_computed_column() -> Result<()> { + let table_scan = test_table_scan_with_struct()?; + let plan = LogicalPlanBuilder::from(table_scan) + .filter(col("id").gt(lit(0u32)))? + .project(vec![ + Expr::Negative(Box::new(col("id"))).alias("id"), + col("user"), + ])? + .project(vec![col("id"), leaf_udf(col("user"), "name")])? + .build()?; + + let ctx = OptimizerContext::new().with_max_passes(1); + let optimizer = Optimizer::with_rules(vec![ + Arc::new(ExtractLeafExpressions::new()), + Arc::new(PushDownLeafProjections::new()), + ]); + let optimized = optimizer.optimize(plan, &ctx, |_, _| {})?; + + insta::assert_snapshot!(format!("{optimized}"), @r#" + Projection: id, __datafusion_extracted_1 AS leaf_udf(test.user,Utf8("name")) + Projection: (- test.id) AS id, test.user, __datafusion_extracted_1 + Filter: test.id > UInt32(0) + Projection: leaf_udf(test.user, Utf8("name")) AS __datafusion_extracted_1, test.id, test.user + TableScan: test + "#); + Ok(()) + } } diff --git a/datafusion/sqllogictest/test_files/struct.slt b/datafusion/sqllogictest/test_files/struct.slt index 87bbd11c986a4..224f1a750cec7 100644 --- a/datafusion/sqllogictest/test_files/struct.slt +++ b/datafusion/sqllogictest/test_files/struct.slt @@ -1803,3 +1803,72 @@ drop view struct_ctor_view; statement ok drop table struct_ctor_null; + +# Regression test for https://github.com/apache/datafusion/issues/25414. +# `-a AS a` computes a new value under the same name as its input column. +# Leaf expression pushdown moves `s['b']` below that projection. The projection +# that computes `-a` must stay, or the query returns the input column `a`. + +statement ok +create table leaf_same_name(a int, s struct) as values (1, {b: 'x'}), (2, {b: 'y'}); + +# The computed column must survive the pushdown. Line 02 holds `(- t.a) AS a`. +query TT +explain select a, s['b'] from (select -a as a, s from leaf_same_name where a > 0); +---- +logical_plan +01)Projection: (- leaf_same_name.a) AS a, __datafusion_extracted_1 AS leaf_same_name.s[b] +02)--Filter: leaf_same_name.a > Int32(0) +03)----Projection: get_field(leaf_same_name.s, Utf8("b")) AS __datafusion_extracted_1, leaf_same_name.a +04)------TableScan: leaf_same_name projection=[a, s] +physical_plan +01)ProjectionExec: expr=[(- a@1) as a, __datafusion_extracted_1@0 as leaf_same_name.s[b]] +02)--RepartitionExec: partitioning=RoundRobinBatch(4), input_partitions=1 +03)----FilterExec: a@1 > 0 +04)------ProjectionExec: expr=[get_field(s@1, b) as __datafusion_extracted_1, a@0 as a] +05)--------DataSourceExec: partitions=1, partition_sizes=[1] + +# Through a Filter +query IT rowsort +select a, s['b'] from (select -a as a, s from leaf_same_name where a > 0); +---- +-1 x +-2 y + +# Through a Limit +query IT rowsort +select a, s['b'] from (select -a as a, s from leaf_same_name limit 10); +---- +-1 x +-2 y + +# A computed column whose type differs from its input column +query IT rowsort +select a, s['b'] from (select a * 10 as a, s from leaf_same_name limit 10); +---- +10 x +20 y + +# The outer filter must see the computed value +query IT rowsort +select a, s['b'] from (select -a as a, s from leaf_same_name) where a < 0; +---- +-1 x +-2 y + +# The group key must be the computed value +query II rowsort +select a, count(s['b']) from (select -a as a, s from leaf_same_name where a > 0) group by a; +---- +-1 1 +-2 1 + +# Through a Sort +query IT rowsort +select a, s['b'] from (select -a as a, s from leaf_same_name order by a); +---- +-1 x +-2 y + +statement ok +drop table leaf_same_name;