This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 21ada17e140ebe76bba7c3e348305eff651bcaef Author: Steven Talbot <[email protected]> AuthorDate: Tue May 28 14:45:49 2019 -0700 [CALCITE-3097] GROUPING SETS breaks on sets of size > 1 due to precedence issues (Steven Talbot) RelToSqlConverter was converting each composite item in GROUPING SETS into a SqlNodeList when it should be a call to the ROW operator. Close apache/calcite#1239 --- .../calcite/rel/rel2sql/RelToSqlConverter.java | 22 +++++++++++++++------- .../apache/calcite/sql/fun/SqlRollupOperator.java | 2 +- .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 20 ++++++++++++++++++++ .../apache/calcite/sql/parser/SqlParserTest.java | 6 ++++++ 4 files changed, 42 insertions(+), 8 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 5abef41..3f391ee 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 @@ -67,6 +67,7 @@ import org.apache.calcite.sql.fun.SqlSingleValueAggFunction; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.validate.SqlValidatorUtil; +import org.apache.calcite.util.ImmutableBitSet; import org.apache.calcite.util.Pair; import org.apache.calcite.util.Permutation; import org.apache.calcite.util.ReflectUtil; @@ -279,17 +280,24 @@ public class RelToSqlConverter extends SqlImplementor SqlStdOperatorTable.GROUPING_SETS.createCall(SqlParserPos.ZERO, aggregate.getGroupSets().stream() .map(groupSet -> - new SqlNodeList( - groupSet.asList().stream() - .map(key -> - groupKeys.get(aggregate.getGroupSet() - .indexOf(key))) - .collect(Collectors.toList()), - SqlParserPos.ZERO)) + groupItem(groupKeys, groupSet, aggregate.getGroupSet())) .collect(Collectors.toList()))); } } + private SqlNode groupItem(List<SqlNode> groupKeys, + ImmutableBitSet groupSet, ImmutableBitSet wholeGroupSet) { + final List<SqlNode> nodes = groupSet.asList().stream() + .map(key -> groupKeys.get(wholeGroupSet.indexOf(key))) + .collect(Collectors.toList()); + switch (nodes.size()) { + case 1: + return nodes.get(0); + default: + return SqlStdOperatorTable.ROW.createCall(SqlParserPos.ZERO, nodes); + } + } + /** @see #dispatch */ public Result visit(TableScan e) { final SqlIdentifier identifier; diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlRollupOperator.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlRollupOperator.java index 75098a7..8953691 100644 --- a/core/src/main/java/org/apache/calcite/sql/fun/SqlRollupOperator.java +++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlRollupOperator.java @@ -68,7 +68,7 @@ class SqlRollupOperator extends SqlInternalOperator { writer.keyword(keyword); } - private static void unparseCube(SqlWriter writer, SqlCall call) { + private void unparseCube(SqlWriter writer, SqlCall call) { writer.keyword(call.getOperator().getName()); final SqlWriter.Frame frame = writer.startList(SqlWriter.FrameTypeEnum.FUN_CALL, "(", ")"); 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 bd2d119..ebaf443 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 @@ -217,6 +217,26 @@ public class RelToSqlConverterTest { .ok(expectedMySql); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-3097";>[CALCITE-3097] + * GROUPING SETS breaks on sets of size > 1 due to precedence issues</a>, + * in particular, that we maintain proper precedence around nested lists. */ + @Test public void testGroupByGroupingSets() { + final String query = "select \"product_class_id\", \"brand_name\"\n" + + "from \"product\"\n" + + "group by GROUPING SETS ((\"product_class_id\", \"brand_name\")," + + " (\"product_class_id\"))\n" + + "order by 2, 1"; + final String expected = "SELECT \"product_class_id\", \"brand_name\"\n" + + "FROM \"foodmart\".\"product\"\n" + + "GROUP BY GROUPING SETS((\"product_class_id\", \"brand_name\")," + + " \"product_class_id\")\n" + + "ORDER BY \"brand_name\", \"product_class_id\""; + sql(query) + .withPostgresql() + .ok(expected); + } + /** Tests GROUP BY ROLLUP of two columns. The SQL for MySQL has * "GROUP BY ... ROLLUP" but no "ORDER BY". */ @Test public void testSelectQueryWithGroupByRollup() { diff --git a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java index d04bde1..5fc871d 100644 --- a/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java +++ b/core/src/test/java/org/apache/calcite/sql/parser/SqlParserTest.java @@ -1810,6 +1810,12 @@ public class SqlParserTest { + "FROM `EMP`\n" + "GROUP BY GROUPING SETS(`DEPTNO`, (`DEPTNO`, `GENDER`), ())"); + sql("select deptno from emp\n" + + "group by grouping sets ((deptno, gender), (deptno), (), gender)") + .ok("SELECT `DEPTNO`\n" + + "FROM `EMP`\n" + + "GROUP BY GROUPING SETS((`DEPTNO`, `GENDER`), `DEPTNO`, (), `GENDER`)"); + // Grouping sets must have parentheses sql("select deptno from emp\n" + "group by grouping sets ^deptno^, (deptno, gender), ()")
