Skip to content

Conversation

@berkaysynnada
Copy link
Contributor

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:

SELECT SUM(amount) OVER running_window, AVG(amount) OVER running_window
             FROM sales_us
             WINDOW running_window AS (ORDER BY ts)
             ORDER BY ts
             LIMIT 5;

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?

@github-actions github-actions bot added core Core DataFusion crate logical-expr Logical plan and expressions sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels May 23, 2023
@berkaysynnada
Copy link
Contributor Author

berkaysynnada commented May 23, 2023

This PR needs sql-parser 0.34 release also. @alamb I have taken the necessary changes from #6416 regarding the sqlparser update. I wonder what you suggest, should I remove those parts and wait for your PR to be merged, or you prefer to continue with this PR?

@alamb
Copy link
Contributor

alamb commented May 23, 2023

This PR needs sql-parser 0.34 release also. @alamb I have taken the necessary changes from #6416 regarding the sqlparser update. I wonder what you suggest, should I remove those parts and wait for your PR to be merged, or you prefer to continue with this PR?

I recommend we wait for #6416 to merge, but we can review this PR earlier.

@alamb
Copy link
Contributor

alamb commented May 23, 2023

#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
@github-actions github-actions bot removed the logical-expr Logical plan and expressions label May 23, 2023
Copy link
Contributor

@comphead comphead left a 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

@alamb
Copy link
Contributor

alamb commented May 23, 2023

I plan to review this tomorrow

Copy link
Contributor

@alamb alamb left a 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!

@mustafasrepo mustafasrepo merged commit a4233ad into apache:main May 25, 2023
@berkaysynnada berkaysynnada deleted the feature/named-window branch May 25, 2023 10:22
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again @berkaysynnada

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for named window frames

5 participants