You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2026/09/17 14:20:37 UTC
[PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
adriangb opened a new pull request, #25412:
URL: https://github.com/apache/datafusion/pull/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 name the input schema holds
more than once resolves to nothing, because no single spelling is correct there.
## Are these changes tested?
Yes. The statement above is added to `struct.slt`. It fails on `main` with the
error above and passes with this change.
`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.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#discussion_r4088747354
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -637,6 +637,24 @@ impl<'a> LeafExpressionExtractor<'a> {
}
}
+/// The way `schema` names `col`, or `None` when it does not hold it unambiguously.
+///
+/// A qualified column is taken as it stands. An unqualified one is matched on name alone
+/// and comes back carrying the qualifier the schema gives that field, so a column pushed
+/// into a projection reads as the input spells it. A name the schema holds more than once
+/// resolves to nothing rather than to an arbitrary one of them.
+fn resolve_against(schema: &DFSchema, col: &Column) -> Option<Column> {
+ match &col.relation {
+ Some(relation) => schema
+ .has_column_with_qualified_name(relation, &col.name)
+ .then(|| col.clone()),
+ None => schema
+ .qualified_field_with_unqualified_name(&col.name)
+ .ok()
+ .map(|(qualifier, field)| Column::new(qualifier.cloned(), field.name())),
+ }
+}
Review Comment:
**Suggestion 1: one lookup for both arms.** `DFSchema::qualified_field_from_column` already does the qualified and the unqualified lookup, and `Column: From<(Option<&TableReference>, &FieldRef)>` exists.
| | Current | Suggested |
|---|---|---|
| Qualified arm | `has_column_with_qualified_name` (exact `TableReference::eq`), returns the caller's spelling | `index_of_column_by_name` (`resolved_eq`), returns the schema's spelling |
| Unqualified arm | `qualified_field_with_unqualified_name` | same |
| Result | two spellings can still differ (for example `public.t.c` vs `t.c`) | always the input's spelling, so `existing_cols.contains` compares like with like |
I ran the optimizer tests with this change: 57 passed.
The doc also claims a duplicate name resolves to `None`. When exactly one of the matches is unqualified, the lookup returns that one (see `qualified_field_with_unqualified_name`). The suggested doc says what the code does.
```suggestion
/// The way `schema` names `col`, or `None` when `schema` does not hold it
/// or the name is ambiguous.
///
/// The result always carries the qualifier the schema gives the field, so
/// two spellings of the same input column compare equal, and a column pushed
/// into a projection reads as the input spells it.
fn resolve_against(schema: &DFSchema, col: &Column) -> Option<Column> {
schema.qualified_field_from_column(col).ok().map(Column::from)
}
```
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -702,27 +720,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.
+ //
+ // Both sides of that check are read in the input's own spelling. A column can
+ // arrive here unqualified while the input names it `t.c`, and the other way round:
+ // a projection of bare names over a qualified input is what eliminating one side
+ // of a union leaves behind. Resolving both sides lets a pass-through the
+ // projection already carries match the one about to be added, and pushes the one
+ // that is genuinely new under the name the input gives it. Left unresolved, the
+ // merged projection would hold `t.c` and a bare `c` together, which
+ // `Projection::try_new` rejects as ambiguous. A same-name alias such as
+ // `t.c AS c` is a pass-through as well, and counts the same as a bare `t.c`.
Review Comment:
**Suggestion 2 (nit): shorter comment.** The existing paragraph above is 5 lines; this one is 9. The same content fits in fewer lines:
```suggestion
// Compare both sides in the input's spelling (see `resolve_against`).
// Without this, a bare `c` and a qualified `t.c` do not match, and the
// merged projection holds both, which `Projection::try_new` rejects as
// ambiguous. Eliminating the empty side of a union makes this shape.
// A same-name alias (`t.c AS c`) counts as a pass-through of `t.c`.
```
##########
datafusion/sqllogictest/test_files/struct.slt:
##########
@@ -1803,3 +1803,28 @@ drop view struct_ctor_view;
statement ok
drop table struct_ctor_null;
+
+# 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');
Review Comment:
**Suggestion 3: lock the plan shape.** The result check passes if a future change to union elimination stops making this shape. Then the test no longer covers the merge. An `EXPLAIN` makes that visible. I ran this block with `--complete` on the PR head, and then without `--complete`: it passes.
```suggestion
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;
```
Optional: `projection_pushdown.slt` already holds the other leaf-pushdown plan tests, so this block can also go there.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#issuecomment-5716107004
cc @pepijnve since you're working on these same files
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #25412:
URL: https://github.com/apache/datafusion/pull/25412
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#discussion_r4038058795
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -702,27 +720,36 @@ 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.
+ //
+ // Both sides of that check are read in the input's own spelling. A column can
+ // arrive here unqualified while the input names it `t.c`, and the other way round:
+ // a projection of bare names over a qualified input is what eliminating one side
+ // of a union leaves behind. Resolving both sides lets a pass-through the
+ // projection already carries match the one about to be added, and pushes the one
+ // that is genuinely new under the name the input gives it. Left unresolved, the
+ // merged projection would hold `t.c` and a bare `c` together, 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())
+ resolve_against(input_schema, c)
Review Comment:
This set ignores trivial pass-through aliases such as `p.c AS c`, even though `passthrough_column` explicitly treats them as pass-throughs. When `columns_needed` contains `c`, the replacement map resolves it to `p.c`; omitting the retained alias here then appends `p.c` beside the existing unqualified `c`, recreating the same ambiguous schema. Normalize every `passthrough_column(e)` here, and add a regression case with a same-name alias.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#discussion_r4098705235
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -702,27 +715,27 @@ 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.
+ //
+ // Compare both sides in the input's spelling (see `resolve_against`).
Review Comment:
What are the 'sides' being referred to here?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#discussion_r4098696313
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -637,6 +637,19 @@ impl<'a> LeafExpressionExtractor<'a> {
}
}
+/// The way `schema` names `col`, or `None` when `schema` does not hold it
Review Comment:
nit: this sentence took me a couple of tries to grok. The function has an active name (an action), but the first sentence is describing a noun (a property). Might be better to write this in a 'Does computation xyz' style.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#issuecomment-6174590977
Thanks @kosiew
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#discussion_r4266755665
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -737,27 +751,33 @@ 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` and the columns in
+ // `columns_needed` against the input schema before comparing them (see
+ // `resolve_against`). Without this, a bare `c` in one set and a
+ // qualified `t.c` in the other do not match, so the merged projection
+ // holds both, and `Projection::try_new` rejects it as ambiguous.
+ //
+ // This shape occurs when the optimizer removes the empty input of a
+ // `UNION ALL` (for example `... UNION ALL SELECT ... WHERE 1 = 2`): the
+ // projection that remains lists bare column names over a qualified
+ // input. A same-name alias (`t.c AS c`) counts as a pass-through of
+ // `t.c`.
+ 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)
{
Review Comment:
Could we add a test where the existing projection retains only `test.a` and `columns_needed` requests bare `b`? It would verify that the newly appended column is `test.b`, with qualifier `test` and name `b`, rather than unqualified `b`. The current tests cover deduplication but not this specific case. This is optional and could be done in a follow-up.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#discussion_r4267188966
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -737,27 +751,33 @@ 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` and the columns in
+ // `columns_needed` against the input schema before comparing them (see
+ // `resolve_against`). Without this, a bare `c` in one set and a
+ // qualified `t.c` in the other do not match, so the merged projection
+ // holds both, and `Projection::try_new` rejects it as ambiguous.
+ //
+ // This shape occurs when the optimizer removes the empty input of a
+ // `UNION ALL` (for example `... UNION ALL SELECT ... WHERE 1 = 2`): the
+ // projection that remains lists bare column names over a qualified
+ // input. A same-name alias (`t.c AS c`) counts as a pass-through of
+ // `t.c`.
+ 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)
{
Review Comment:
added in 68a2780
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#discussion_r4038668131
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -702,27 +720,36 @@ 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.
+ //
+ // Both sides of that check are read in the input's own spelling. A column can
+ // arrive here unqualified while the input names it `t.c`, and the other way round:
+ // a projection of bare names over a qualified input is what eliminating one side
+ // of a union leaves behind. Resolving both sides lets a pass-through the
+ // projection already carries match the one about to be added, and pushes the one
+ // that is genuinely new under the name the input gives it. Left unresolved, the
+ // merged projection would hold `t.c` and a bare `c` together, 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())
+ resolve_against(input_schema, c)
Review Comment:
Fixed in 5e7ec7a266: the existing side now collects `passthrough_column(e)`, so `t.c AS c` counts the same as a bare `t.c`. Added `test_merge_into_projection_with_same_name_alias`, which fails without the change with `AmbiguousReference { test.a }`. A same-name alias case in SQL does not reach this path (the planner drops `env AS env` first), so the regression test is a unit test.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#issuecomment-5716371975
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25412?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 82.32%. Comparing base ([`a2b8093`](https://app.codecov.io/gh/apache/datafusion/commit/a2b809331f70bb5468709c5f318cf0882f63e1e4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`029801c`](https://app.codecov.io/gh/apache/datafusion/commit/029801c8d81e0b0a503e9dde3afbe9313861823a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #25412 +/- ##
=======================================
Coverage 82.32% 82.32%
=======================================
Files 1137 1137
Lines 431824 431834 +10
Branches 431824 431834 +10
=======================================
+ Hits 355500 355509 +9
+ Misses 54838 54835 -3
- Partials 21486 21490 +4
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25412?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#issuecomment-5805673084
@pepijnve if you are able to review this it would be great, we are hitting this in prod so pretty keen to fix it. I also think it's a good starting place to tackle the EPIC we have now.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: resolve pass-through columns against the input when merging an extraction projection [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25412:
URL: https://github.com/apache/datafusion/pull/25412#discussion_r4098705235
##########
datafusion/optimizer/src/extract_leaf_expressions.rs:
##########
@@ -702,27 +715,27 @@ 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.
+ //
+ // Compare both sides in the input's spelling (see `resolve_against`).
Review Comment:
What are the 'sides' being referred to here? In the final sentence there's a reference to an empty union side. Makes this hard to interpret.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org