Conversation
| # End agg.iq | ||
|
|
||
| # [CALCITE-6537] Add syntax to allow non-aggregated rows to be used in GROUPING SETS | ||
| # non-aggregated (detail) rows. Requires LENIENT conformance. |
There was a problem hiding this comment.
Since this is not standard SQL and is not supported by mainstream databases, it was converted to equivalent PostgreSQL syntax for verification; the results matched expectations: https://onecompiler.com/postgresql/44yz6y5r4
| * | ||
| * @see org.apache.calcite.sql.validate.SqlConformance#isGroupingSetsStarAllowed() | ||
| */ | ||
| class GroupingSetsStarTest { |
There was a problem hiding this comment.
If these tests duplicate the quidem tests, they are not necessary
There was a problem hiding this comment.
Yes you are right, I had removed the GroupingSetsStarTest.
|
|
||
| # End agg.iq | ||
|
|
||
| # [CALCITE-6537] Add syntax to allow non-aggregated rows to be used in GROUPING SETS |
There was a problem hiding this comment.
the parser uses a stack for grouping star, so it seems to support nested * calls. Are there unit tests for these cases?
We should either have unit tests for the full language supported, or support a more restricted form of the language.
There was a problem hiding this comment.
Make sensen, I chose the restricted form. The parser is unchanged (the stack is still needed for grouping constructs nested in GROUPING SETS, e.g. GROUPING SETS (ROLLUP (deptno, *))), but the validator now only expands * in the well-defined positions of the syntax: a complete grouping set (GROUPING SETS (*)), an element of a grouping set (GROUPING SETS ((deptno), (*))), or an argument of ROLLUP/CUBE (ROLLUP (deptno, *)), including the GROUP BY DISTINCT wrapper. A star nested inside other expressions, say GROUPING SETS (ABS(*)), parses but is not expanded, so validation fails with "Unknown identifier ''", and top-level GROUP BY * remains a parse error.
|
Thank you for the review and so sorry, I saw this comment a bit late @mihaibudiu . |
|
mihaibudiu
left a comment
mihaibudiu
left a comment
There was a problem hiding this comment.
This looks fine, but it's subtle.
2 participants
jira: https://issues.apache.org/jira/browse/CALCITE-6537