-
Notifications
You must be signed in to change notification settings - Fork 550
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
feat: Group By All #964
feat: Group By All #964
Conversation
Hi @alamb, could you please take a look? The Datafusion part is also set, and once this PR is merged, I'll be able to open its corresponding PR. |
I will do so -- thanks @berkaysynnada |
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.
Looks good to me -- thank you @berkaysynnada
Co-authored-by: Andrew Lamb <[email protected]>
looks like there was a logical conflict on main. I will fix that Update: @ding-young fixed it in #965 I think it you merge up from main this PR will pass CI |
Thanks @berkaysynnada !@ |
Co-authored-by: Andrew Lamb <[email protected]>
There exists a GROUP BY usage with ALL keyword in some platforms (https://docs.snowflake.com/en/sql-reference/constructs/group-by#label-group-by-all-columns, https://duckdb.org/docs/sql/query_syntax/groupby.html).
It groups together all the columns in the SELECT statement that aren't enclosed within aggregate functions. This streamlines the syntax by keeping the list of columns in one place and helps avoid errors by ensuring that the level of detail in your SELECT matches the grouping in your GROUP BY clause.
When this PR is merged, Datafusion part will be opened. Exclude support will also be on the following PR's.