-
Notifications
You must be signed in to change notification settings - Fork 655
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
fix(starrocks): exp.Unnest transpilation #3966
Conversation
0267692
to
640411b
Compare
I tried to add the logic of setting default value when there is no table alias, PTAL @georgesittas @VaggelisD I need to transpile the starrocks sql:
Expected spark sql:
|
be0a7af
to
d2119e2
Compare
d2119e2
to
77b87d5
Compare
I found that when unnest has multiple outputs, it will be transpiled into multiple Lateral view statements (actually incorrect), as shown in the following SQL of
Transpiled into
Incorrect reason: In fact, our users often use the syntax combination table function
Expected output:
But function I added a commit. Will be open to any suggestions =) |
08b0bc1
to
6eac225
Compare
use `arrays_zip` to merge multiple `Lateral view explode`
6eac225
to
8261ff5
Compare
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.
Hey, thanks for the follow-up PR! Left a few comments in my first pass.
b749827
to
c1bfdba
Compare
c1bfdba
to
6fa1ae7
Compare
@VaggelisD Added new commit, PTAL |
@VaggelisD Good evening, I made some change, please take a look at it when you have time |
5759bfa
to
6676b89
Compare
6676b89
to
2b0179a
Compare
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 for the PR @hellozepp!
Co-authored-by: Jo <[email protected]>
@georgesittas @VaggelisD Added new commit, PTAL |
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 for the PR @hellozepp - LGTM. Appreciate the patience & quick iterations.
We plan to get this in soon and take a stab at refactoring the hive transformation so that it actually does the right thing. One way to go about this is by simulating the INLINE
& ARRAYS_ZIP
with a subquery that joins the POSEXPLODE
d arrays on their positions and then projects the corresponding columns so they can be accessed in the scope of the original UNNEST
.
@georgesittas Agree. |
Sorry, I cleaned up my sqlglot repository because it was a mess. Wait a minute and I'll make another commit. There won't be any new code since our last commit. |
Fixes #3962
This PR adds support for the following:
exp.UNNEST
to include "unnest" as the default table alias if not specifiedexp.Inline