-
Notifications
You must be signed in to change notification settings - Fork 606
Support HAVING/LIMIT/OFFSET/FETCH without FROM and other follow-ups #116
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
Conversation
Postgres allows it, as does ANSI SQL per the <query expression> definition: https://jakewheat.github.io/sql-overview/sql-2011-foundation-grammar.html#_7_13_query_expression
...which is weird but allowed: https://jakewheat.github.io/sql-overview/sql-2011-foundation-grammar.html#table-expression https://dba.stackexchange.com/a/57453/15599 Also add a test for GROUP BY .. HAVING
Pull Request Test Coverage Report for Build 352
💛 - Coveralls |
Pull Request Test Coverage Report for Build 351
💛 - Coveralls |
src/dialect/keywords.rs
Outdated
]; | ||
|
||
/// Can't be used as a column alias, so that `SELECT <expr> alias` | ||
/// can be parsed unambiguously without looking ahead. | ||
pub const RESERVED_FOR_COLUMN_ALIAS: &[&str] = &[ | ||
// Reserved as both a table and a column alias: | ||
WITH, SELECT, WHERE, GROUP, ORDER, UNION, EXCEPT, INTERSECT, | ||
WITH, SELECT, WHERE, GROUP, ORDER, LIMIT, OFFSET, FETCH, UNION, EXCEPT, INTERSECT, |
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.
I know this isn't the fault of this commit, but it's a shame that these keywords have to be duplicated between the two constants. I don't have any obvious solutions, and definitely doesn't need to be considered in this PR; just thought I'd flag it in case you happened to have any ideas.
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.
I don't like it either, but the solutions seemed too complex for a trivial problem...
These lists will become dialect-specific at some point, and we would be able to build the reserved lists in the dialect constructor then, I guess.
#68 made me review the keywords in
RESERVED_FOR_TABLE_ALIAS
andRESERVED_FOR_COLUMN_ALIAS
, and I realized more of them need to be "Reserved as both a table and a column alias".Also after
TableAlias
was introduced in #98 to representalias [ (col1, col2, ...) ]
as part of a TableFactor, it makes sense to re-use it inCte
.