This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 9f91d611f52e97681882e8aaa4e974917df7fb13
Author: xiejiajun <[email protected]>
AuthorDate: Sun Dec 4 14:35:49 2022 +0800

    [CALCITE-5416] JDBC adapter for MySQL 5 incorrectly combines GROUP BY 
ROLLUP and ORDER BY clauses
    
    JDBC adapter should convert
    
      SELECT ...
      GROUP BY ROLLUP(x, y)
      ORDER BY y, x
    
    to
    
      SELECT ...
      FROM (SELECT ...
        GROUP BY x, y WITH ROLLUP)
      ORDER BY y, x
    
    because MySQL 5 does not allow GROUP BY WITH ROLLUP and
    ORDER BY in the same query.
    
    Close apache/calcite#2997
---
 .../calcite/rel/rel2sql/RelToSqlConverter.java     | 41 ++++++++++++++++------
 .../calcite/rel/rel2sql/RelToSqlConverterTest.java |  6 ++--
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java 
b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
index 41ca2e2290..b8a3aaa372 100644
--- a/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
+++ b/core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java
@@ -103,7 +103,6 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Deque;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -873,19 +872,39 @@ public class RelToSqlConverter extends SqlImplementor
       if (hasTrickyRollup(e, aggregate)) {
         // MySQL 5 does not support standard "GROUP BY ROLLUP(x, y)", only
         // the non-standard "GROUP BY x, y WITH ROLLUP".
-        // It does not allow "WITH ROLLUP" in combination with "ORDER BY",
-        // but "GROUP BY x, y WITH ROLLUP" implicitly sorts by x, y,
+        List<Integer> rollupList =
+            Aggregate.Group.getRollup(aggregate.getGroupSets());
+        List<Integer> sortList = e.getCollation()
+            .getFieldCollations()
+            .stream()
+            .map(f -> aggregate.getGroupSet().nth(f.getFieldIndex()))
+            .collect(Collectors.toList());
+        // "GROUP BY x, y WITH ROLLUP" implicitly sorts by x, y,
         // so skip the ORDER BY.
-        final Set<Integer> groupList = new LinkedHashSet<>();
-        for (RelFieldCollation fc : e.collation.getFieldCollations()) {
-          groupList.add(aggregate.getGroupSet().nth(fc.getFieldIndex()));
-        }
-        groupList.addAll(Aggregate.Group.getRollup(aggregate.getGroupSets()));
+        final boolean isImplicitlySort = Util.startsWith(rollupList, sortList);
         final Builder builder =
-            visitAggregate(aggregate, ImmutableList.copyOf(groupList),
+            visitAggregate(aggregate, rollupList,
                 Clause.GROUP_BY, Clause.OFFSET, Clause.FETCH);
-        offsetFetch(e, builder);
-        return builder.result();
+        Result result = builder.result();
+        if (sortList.isEmpty()
+            || isImplicitlySort) {
+          offsetFetch(e, builder);
+          return result;
+        }
+        // MySQL does not allow "WITH ROLLUP" in combination with "ORDER BY",
+        // so generate the grouped result apply ORDER BY to it.
+        SqlSelect sqlSelect = result.subSelect();
+        SqlNodeList sortExps = exprList(builder.context, e.getSortExps());
+        sqlSelect.setOrderBy(sortExps);
+        if (e.offset != null) {
+          SqlNode offset = builder.context.toSql(null, e.offset);
+          sqlSelect.setOffset(offset);
+        }
+        if (e.fetch != null) {
+          SqlNode fetch = builder.context.toSql(null, e.fetch);
+          sqlSelect.setFetch(fetch);
+        }
+        return result(sqlSelect, ImmutableList.of(Clause.ORDER_BY), e, null);
       }
     }
     if (e.getInput() instanceof Project) {
diff --git 
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java 
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index ca715e2033..b7469c20af 100644
--- 
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++ 
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -696,9 +696,11 @@ class RelToSqlConverterTest {
         + "FROM \"foodmart\".\"product\"\n"
         + "GROUP BY ROLLUP(\"product_class_id\", \"brand_name\")\n"
         + "ORDER BY \"brand_name\", \"product_class_id\"";
-    final String expectedMysql = "SELECT `product_class_id`, `brand_name`\n"
+    final String expectedMysql = "SELECT *\n"
+        + "FROM (SELECT `product_class_id`, `brand_name`\n"
         + "FROM `foodmart`.`product`\n"
-        + "GROUP BY `brand_name`, `product_class_id` WITH ROLLUP";
+        + "GROUP BY `product_class_id`, `brand_name` WITH ROLLUP) AS `t0`\n"
+        + "ORDER BY `brand_name`, `product_class_id`";
     sql(query)
         .ok(expected)
         .withMysql().ok(expectedMysql);

Reply via email to