[calcite] branch main updated: [CALCITE-5518] RelToSql converter generates invalid order of ROLLUP fields

[jiajunxie]

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

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


The following commit(s) were added to refs/heads/main by this push:
     new 90599a6e9b [CALCITE-5518] RelToSql converter generates invalid order 
of ROLLUP fields
90599a6e9b is described below

commit 90599a6e9b0143eb3d9175af7b5ff374e1b95252
Author: xiejiajun <jiajunbernou...@foxmail.com>
AuthorDate: Sun Feb 12 15:34:14 2023 +0800

    [CALCITE-5518] RelToSql converter generates invalid order of ROLLUP fields
---
 .../calcite/rel/rel2sql/RelToSqlConverter.java     |  7 +++++-
 .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 28 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

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 b8747c1a35..efcdc0c93f 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
@@ -620,8 +620,13 @@ public class RelToSqlConverter extends SqlImplementor
       // a singleton CUBE and ROLLUP are the same but we prefer ROLLUP;
       // fall through
     case ROLLUP:
+      final List<Integer> rollupBits = 
Aggregate.Group.getRollup(aggregate.groupSets);
+      final List<SqlNode> rollupKeys = rollupBits
+          .stream()
+          .map(bit -> builder.context.field(bit))
+          .collect(Collectors.toList());
       return ImmutableList.of(
-          SqlStdOperatorTable.ROLLUP.createCall(SqlParserPos.ZERO, groupKeys));
+          SqlStdOperatorTable.ROLLUP.createCall(SqlParserPos.ZERO, 
rollupKeys));
     default:
     case OTHER:
       // Make sure that the group sets contains all bits.
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 83f1fd13d1..7016a74b84 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
@@ -707,6 +707,34 @@ class RelToSqlConverterTest {
         .withMysql().ok(expectedMysql);
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-5518";>[CALCITE-5518]
+   * RelToSql converter generates invalid order of ROLLUP fields</a>.
+   */
+  @Test void testGroupingSetsRollupNonNaturalOrder() {
+    final String query1 = "select \"product_class_id\", \"brand_name\"\n"
+        + "from \"product\"\n"
+        + "group by GROUPING SETS ((\"product_class_id\", \"brand_name\"),"
+        + " (\"brand_name\"), ())\n";
+    final String expected1 = "SELECT \"product_class_id\", \"brand_name\"\n"
+        + "FROM \"foodmart\".\"product\"\n"
+        + "GROUP BY ROLLUP(\"brand_name\", \"product_class_id\")";
+    sql(query1)
+        .withPostgresql().ok(expected1);
+
+    final String query2 = "select \"product_class_id\", \"brand_name\", 
\"product_id\"\n"
+        + "from \"product\"\n"
+        + "group by GROUPING SETS ("
+        + " (\"product_class_id\", \"brand_name\", \"product_id\"),"
+        + " (\"product_class_id\", \"brand_name\"),"
+        + " (\"brand_name\"), ())\n";
+    final String expected2 = "SELECT \"product_class_id\", \"brand_name\", 
\"product_id\"\n"
+        + "FROM \"foodmart\".\"product\"\n"
+        + "GROUP BY ROLLUP(\"brand_name\", \"product_class_id\", 
\"product_id\")";
+    sql(query2)
+        .withPostgresql().ok(expected2);
+  }
+
   /** Tests a query with GROUP BY and a sub-query which is also with GROUP BY.
    * If we flatten sub-queries, the number of rows going into AVG becomes
    * incorrect. */