[GitHub] [calcite] libenchao commented on pull request #2994: [CALCITE-5411] Updated Spark dialect to reflect support of ROLLUP and CUBE standard syntax

["libenchao (via GitHub)" <gi...@apache.org>]

libenchao commented on PR #2994:
URL: https://github.com/apache/calcite/pull/2994#issuecomment-1484446777

   > @libenchao
   > 
   > First reason is that syntax "WITH ROLLUP" is non standard, while "GROUP BY ROLLUP(...)" syntax is standard.
   > 
   > Second reason is following:
   > 
   > * We have a [hasTrickyRollup](https://github.com/apache/calcite/blob/01002b4a16ddc2bdb7aa59a5f5f22649111d2ea9/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java#L984) check that has a meaning of something like "dialect supports WITH ROLLUP, but not GROUP BY ROLLUP".
   > * In our code for `Sort` implementation we assume that we are not allowed to have "WITH ROLLUP" clause followed by "ORDER" by clause. That is actually not true for the Spark dialect, but true for MySQL5. And that constraint can make generated SQL code for Spark more complicated that is actually required (two SQL SELECT clause will be needed).
   > * Now we have a solution: either to fix dialect attributes of Spark dialect so that we always generate "GROUP BY ROLLUP(..)" or we need to implement more fine-grained distinction between "WITH ROLLUP supporting ORDER BY" and "WITH ROLLUP not supporing ORDER BY"
   > * IMO, first solution looks less intrusive for current code base
   
   Sounds good to me, standard syntax is better since Spark supports it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org