-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Allow flag to alias all projected substrait expressions with a UUID #19123
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
base: main
Are you sure you want to change the base?
Allow flag to alias all projected substrait expressions with a UUID #19123
Conversation
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn alias_all_expressions_flag() -> Result<()> { |
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.
Happy to take ideas on the best way to write this test but I wanted to show that the UUIDs actually light up correctly.
5541990 to
1f51265
Compare
|
@LiaCastaneda thought you might have some ideas here |
|
👋 Sorry for the late reply, |
|
I'm happy if there's a better route forward we're just looking for a quick fix without having to fork the whole substrait conversation. |
|
Had a go at the fix here #19856 |
Which issue does this PR close?
Rationale for this change
As previously discussed in #17299. This continues to be a bit of a thorn for expressions that are the same on the left and the right side of the of a join leading to ambiguous references.
Downside here is that it makes the plan less readable which is why I thought it would be better if this was a config that users can opt into to make things more stable. Having said that in my experience the plans created via substrait are super hard to read anyway. If you have a deeply nested CAST statement for example then the name that appears in the plan is unintelligible anyway.
What changes are included in this PR?
Adds
substrait_alias_all_expressionsconfig which applies a UUID alias to all expressions during substrait conversion.Are these changes tested?
Yes
Are there any user-facing changes?