-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Strip table qualifiers from schema in UNION ALL
for unparser
#11082
Strip table qualifiers from schema in UNION ALL
for unparser
#11082
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @phillipleblanc -- I left some style comments / suggestions, but I think this PR does what it says it does and could be merged as is
Thank you very much for the contribution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Thanks again @phillipleblanc and nice work!@ |
…e#11082) * Add test cases for issue * Remove test from logical_plan/builder.rs * Remove table qualifiers for sorts following a Union * Use transform_up and add more documentation on why this is needed
Which issue does this PR close?
Closes #10706
Rationale for this change
The schema that is the result of a UNION ALL should not have any table qualifiers, as the table information has effectively been erased and is no longer a valid reference.
Consider the following SQL:
The logical schema from this UNION ALL should be just
foo
, but it is currently taking the first input's schema directly, resulting in a schema oftable1.foo
.This came up as an issue when trying to validate the unparser works correctly for UNION ALL statements, which I added in #10603
Slightly modifying the above example, if I add an ORDER BY clause to the input SQL:
Then the resulting unparsed SQL will be:
Because the unparser takes the schema directly from the Union node when writing out the column expressions.
This is my second attempt to fix #10706, my first attempt was to remove the table qualifiers when building the Union plan directly, but that caused issues documented here: #10707 (comment)
This attempt scopes the problem to just removing the table qualifiers during the unparsing.
What changes are included in this PR?
Rewrite the LogicalPlan before unparsing to remove table identifiers from the Union All plan and any sort expressions that take a Union All plan as input.
Are these changes tested?
Yes, I added a test in the Unparser
sql_integration
test.Are there any user-facing changes?
No