adriangb opened a new pull request, #26871: URL: https://github.com/apache/datafusion/pull/26871
## Which issue does this PR close? - Part of https://github.com/apache/datafusion/issues/25448 (step 1). - Part of EPIC https://github.com/apache/datafusion/issues/25459. ## Rationale for this change The leaf-pushdown rules (`ExtractLeafExpressions`, `PushDownLeafProjections`) match columns by name. A bare `c` and a qualified `t.c` can name the same field but do not compare equal. Two fields with the same name can be confused. One result: when a merge into an existing projection looks for an extraction that is already there, the lookup misses if the two spell the column differently. The rule then adds the same extraction a second time. With the same alias, `Projection::try_new` fails with `DuplicateUnqualifiedField`. This is the qualified/unqualified path of https://github.com/apache/datafusion/issues/25447. ## What changes are included in this PR? One resolver, `column_index(schema, col) -> Option<usize>` (`None` when the column is missing or ambiguous). All decisions below compare indices. | Site | Before | After | | --- | --- | --- | | `remap_pairs_and_columns` (SubqueryAlias, Union) | `replace_cols_by_name` with a `qualified_name` map | remap by position | | Merge: resolve through the projection | `build_projection_replace_map` (`HashMap<String, Expr>`) | `projection_definition(proj, idx)`, by output index | | Merge: find an existing extraction | `Expr` equality, as spelled | `(Expr, alias)` set, `Expr` in the canonical spelling of the input schema | | Merge: pass-through columns | name sets (`output_names`, `existing_cols`) | `column_index` in the output schema, then `has_column_with_unqualified_name` | | `resolve_against` | `qualified_field_from_column` (first match) | `column_index` (rejects ambiguous) | | `volatile_output_columns` / `would_duplicate_volatile` | `flat_name` sets | index sets | | `merge_would_duplicate_kept_expr` | `flat_name` counts | index counts | The merge also no longer adds an extraction a second time when the existing projection already computes it under two aliases. Still by name (follow-ups): - Reuse of an existing extraction under another alias (the main MRE of https://github.com/apache/datafusion/issues/25447). The merge still duplicates the computation. That issue is assigned to @buraksenn, so this PR leaves it alone. - The recovery projection (matches output fields by name). - `PushDownFilter` (`replace_cols_by_name` there is not changed). ## What is the testing strategy for this PR? - New unit tests in `extract_leaf_expressions.rs`: - `test_merge_finds_extraction_with_other_column_spelling`: existing projection has `leaf_udf(a) AS __datafusion_extracted_1`, caller asks for `leaf_udf(test.a)` under the same alias. Fails on `main` with `DuplicateUnqualifiedField`, passes here. - `test_merge_finds_extraction_computed_under_two_aliases`. - One snapshot changes: in the merge test with `test.c + 1 AS d`, the extraction now prints as `leaf_udf(test.a, ...)` instead of `leaf_udf(a, ...)`. The merge now writes columns in the canonical spelling of the input schema. Same plan, same result. - No `.slt` changes. - `cargo test -p datafusion-optimizer`, all sqllogictests, and the leaf-pushdown differential fuzz test (`LEAF_PUSHDOWN_FUZZ_CASES=2000`) pass. ## Are there any user-facing changes? No. Internal refactor of two optimizer rules. A merge that hits the qualified/unqualified mismatch no longer adds a duplicate extraction. 🤖 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]
