Repository navigation
Conversation
…p key shares its name build_common_expr_project_plan deduplicated input fields by their flat qualified name, so the unqualified `t.a` field produced by a `CAST(a AS VARCHAR)` group key hid the qualified `t.a` column and optimize_projections failed with "No field named t.a" (or panicked). Deduplicate by Column instead. Closes apache#26621
github-actions
Bot
added
optimizer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
GROUP BY CAST(a AS VARCHAR), awith a common subexpression fails inoptimize_projections("No field named t.a", sometimes a panic) #26621.Rationale for this change
SELECT a, a + 1, (a + 1) * 2 FROM t GROUP BY CAST(a AS VARCHAR), afails to plan withOptimizer rule 'optimize_projections' failed ... No field named t.a, and a close variant (COALESCE(min('x'), CAST(a AS VARCHAR))in the select list) panics inoptimize_projectionsabout half the time. Without the CAST group key the same query works.What changes are included in this PR?
The aggregate outputs an unqualified field named
t.a(the CAST key) next to the qualifiedt.a.build_common_expr_project_planincommon_subexpr_eliminate.rsdeduplicated the input fields by their flat name,qualified_name(qualifier, name), which is"t.a"for both, so the qualified column was left out of the intermediate projection while the outer projection still referenced it.The dedup set now holds
Columns (qualifier and name), so the two fields stay distinct. The common expression aliases go in as unqualified columns, which still keeps an input field with the same unqualified name from being projected twice.What is the testing strategy for this PR?
Added both queries from the issue to
cse.slt. Without the change the new cases fail with theFieldNotFoundpanic from the issue; with it they return1 2 4and1 2 x.Ran locally:
cargo test -p datafusion-sqllogictest --test sqllogictests: all 540 files passcargo test -p datafusion-optimizer: 987 passedcargo fmt --all -- --check,cargo clippy -p datafusion-optimizer --all-targets --all-features -- -D warningsAre there any user-facing changes?
Queries like the ones above now plan and return results instead of erroring. No API changes.