adriangb opened a new pull request, #25445: URL: https://github.com/apache/datafusion/pull/25445
## Which issue does this PR close? - Closes https://github.com/apache/datafusion/issues/25414. ## Rationale for this change A subquery that computes a column under the same name as its input column gives wrong results. This happens with the default configuration. ```sql create table t(a int, s struct<b varchar>) as values (1, {b: 'x'}), (2, {b: 'y'}); select a, s['b'] from (select -a as a, s from t where a > 0); ``` | Query | DataFusion on `main` | DuckDB and PostgreSQL | |---|---|---| | `select a, s['b'] from (select -a as a, s from t where a > 0)` | `1 x` / `2 y` | `-1 x` / `-2 y` | | `select a, s['b'] from (select -a as a, s from t limit 10)` | `1 x` / `2 y` | `-1 x` / `-2 y` | | `select a, s['b'] from (select a * 10 as a, s from t limit 10)` | `Internal error` | `10 x` / `20 y` | | `select a, s['b'] from (select -a as a, s from t) where a < 0` | `1 x` / `2 y` | `-1 x` / `-2 y` | | `select a, count(s['b']) from (select -a as a, s from t where a > 0) group by a` | groups `1` and `2` | groups `-1` and `-2` | The fourth query returns rows that fail its own filter. The third query stops the plan with this message: ``` Internal error: Assertion failed: compatible: Failed due to a difference in schemas: original schema: ... "a", data_type: Int64 ... new schema: ... "a", data_type: Int32 ... ``` The results are correct with `set datafusion.optimizer.enable_leaf_expression_pushdown = false;`. The cause is in `split_and_push_projection` in `datafusion/optimizer/src/extract_leaf_expressions.rs`. The rule pushes the extraction of `s['b']` below the projection that computes `-a AS a`. It then decides if it must keep a recovery projection, and it decides from the set of unqualified field names of the pushed plan. A name says nothing about the value behind it. For this projection: ```text Projection: (- t.a) AS a, t.s, get_field(t.s, "b") AS __datafusion_extracted_1 Filter: t.a > Int32(0) TableScan: t ``` the pushed plan holds the names `a`, `s` and `__datafusion_extracted_1` again, but its `a` is the table column `t.a`, not `- t.a`. The two sets are equal, the rule drops the recovery projection, and the computed column becomes its own input column: ```text Projection: a, __datafusion_extracted_1 AS t.s[b] Filter: t.a > Int32(0) Projection: get_field(t.s, "b") AS __datafusion_extracted_1, t.a, t.s TableScan: t ``` The same comparison ignores data types. That is why `a * 10 AS a` fails the optimizer schema check instead of giving wrong data. ## What changes are included in this PR? 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. `passthrough_column` gives that answer, and it already accepts a requalification such as `t.a AS a`. The name comparison stays as it is. It catches the leaked-column case that it was written for, which the expression check does not see. The comparison still ignores qualifiers, so the `SubqueryAlias` requalification behaviour does not change. The change is inside `split_and_push_projection` only. https://github.com/apache/datafusion/pull/25412 touches `build_extraction_projection_impl` in the same file. The two changes are independent and fix different bugs. The only overlap is that both PRs append a block to the end of `datafusion/sqllogictest/test_files/struct.slt`, so the second one to merge needs a trivial rebase there. ## What is the testing strategy for this PR? Six SQL shapes go in `datafusion/sqllogictest/test_files/struct.slt`: through a `Filter`, through a `Limit`, a computed column with a different data type, an outer filter on the computed column, a group key, and through a `Sort`. One `EXPLAIN` shows that the projection that computes `-a` stays in the plan. A counterfactual run confirms each one. With the fix reverted, the new block gives 6 errors: the `EXPLAIN`, four wrong results and the internal error. The `Sort` shape was already correct and guards it. A unit test, `test_recovery_kept_for_same_name_computed_column` in `extract_leaf_expressions.rs`, builds the plan shape that loses the computed column and runs the two leaf rules alone, in their production order. All 883 `datafusion-optimizer` tests pass. No existing insta snapshot changes. All 520 sqllogictest files pass. ## Are there any user-facing changes? Queries of this shape now give correct results. There are no API changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
