Implement GROUP BY () without columns #1218
Conversation
|
Which database does support this? Whats the meaning of this? A group by that does nothing? |
|
Fixes Issue #1176. I did not want to believe it myself, but it is supported by Postgres and H2. I published the valid samples. |
| @@ -2326,9 +2326,15 @@ GroupByElement GroupByColumnReferences(): | |||
| } | |||
| { | |||
| <K_GROUP> <K_BY> | |||
| ( | |||
| columnReference=SimpleExpression() {groupBy.addGroupByExpression(columnReference); } | |||
| ( LOOKAHEAD(3) ( | |||
There was a problem hiding this comment.
I changed a bit of logic in my latest commits. There is now a production SimpleExpressionList with and without brackets. Maybe we should change the group by expressions to an Expression List and simply put a SimpleExpressionList into it. This one should support both cases, but I am not sure about empty brackets.
There was a problem hiding this comment.
No problem, will look into that tonight. You will have it before you wake up.
There was a problem hiding this comment.
Delivered as requested:
<K_GROUP> <K_BY>
( LOOKAHEAD(2) (
"(" ")" { groupBy.withUsingBrackets(true); }
)
| LOOKAHEAD(3) (
"(" list = SimpleExpressionList(true) ")" { groupBy.setGroupByExpressionList(list); }
)
|
( list = SimpleExpressionList(false) { groupBy.setGroupByExpressionList(list); }
)
|
Isn't it a beauty?
There was a problem hiding this comment.
Btw, SimpleExpressionLists can't be empty and always expects at least 1 element in the Grammar Definition. I did not want to refactor this right now, but I think we should look for making it a normal list which could be empty.
Also solves issue JSQLParser#1210 automatically
CLEAN PR against master with minimal change set.