-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Named window support #6419
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
Named window support #6419
Conversation
I recommend we wait for #6416 to merge, but we can review this PR earlier. |
|
#6416 is merged |
# Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
# Conflicts: # datafusion/sql/src/expr/function.rs # datafusion/sql/src/expr/mod.rs # datafusion/sql/src/select.rs # datafusion/sql/src/statement.rs
comphead
left a comment
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.
Great, I'm linking this new syntax to #6338 where is window functions get documented
|
I plan to review this tomorrow |
alamb
left a comment
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 @berkaysynnada and @mustafasrepo -- I think this looks great!
alamb
left a comment
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 @berkaysynnada
Which issue does this PR close?
Closes #6125.
Rationale for this change
With 0.34 release of sql-parser, statements can differentiate the named windows. We can now support such queries:
What changes are included in this PR?
After the query parse, WindowType of the function can be in the form of WindowSpec or NamedWindow. In
select_to_plan()function, if there exist some NamedWindow's, there are reshaped as WindowSpec's by using the definition of the windows. If there are any undefined windows, an error is generated.sqlparser dependency is also updated to 0.34 with this PR.
Are these changes tested?
Yes, slt tests are added for both working and erroneous conditions.
Are there any user-facing changes?