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
The following commit(s) were added to refs/heads/master by this push:
new c7d1818 [CALCITE-4748] If there are duplicate GROUPING SETS, Calcite
should return duplicate rows (NobiGo)
c7d1818 is described below
commit c7d1818763b7433f4ee2f47cc1536180ed5124fb
Author: NobiGo <[email protected]>
AuthorDate: Mon Jul 12 21:02:58 2021 +0800
[CALCITE-4748] If there are duplicate GROUPING SETS, Calcite should return
duplicate rows (NobiGo)
Close apache/calcite#2503
---
.../java/org/apache/calcite/tools/RelBuilder.java | 25 +++-
.../org/apache/calcite/test/RelBuilderTest.java | 12 +-
.../apache/calcite/test/SqlToRelConverterTest.java | 22 ++-
.../apache/calcite/test/SqlToRelConverterTest.xml | 91 +++++++++---
core/src/test/resources/sql/agg.iq | 162 +++++++++++++++++++++
5 files changed, 284 insertions(+), 28 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 6ab8e23..40268b4 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -1934,8 +1934,9 @@ public class RelBuilder {
final ImmutableSortedMultiset<ImmutableBitSet> groupSetMultiset =
ImmutableSortedMultiset.copyOf(ImmutableBitSet.COMPARATOR,
groupSetList);
- if (Iterables.any(aggCalls, RelBuilder::isGroupId)) {
- return rewriteAggregateWithGroupId(groupSet, groupSetMultiset,
+ if (Iterables.any(aggCalls, RelBuilder::isGroupId)
+ || !ImmutableBitSet.ORDERING.isStrictlyOrdered(groupSetMultiset)) {
+ return rewriteAggregateWithDuplicateGroupSets(groupSet,
groupSetMultiset,
ImmutableList.copyOf(aggCalls));
}
groupSets = ImmutableList.copyOf(groupSetMultiset.elementSet());
@@ -2093,14 +2094,26 @@ public class RelBuilder {
* flatten, sorting, redundancy removal), this information is lost in
RelNode.
* Therefore, it is impossible to implement the function in runtime.
*
- * <p>To fill this gap, an aggregation query that contains {@code GROUP_ID()}
- * function will generally be rewritten into UNION when converting to
RelNode.
+ * <p>To fill this gap, an aggregation query that contains duplicate group
+ * sets is rewritten into a Union of Aggregate operators whose group sets are
+ * distinct. The number of inputs to the Union is equal to the maximum number
+ * of duplicates. In the {@code N}th input to the Union, calls to the
+ * {@code GROUP_ID} aggregate function are replaced by the integer literal
+ * {@code N}.
+ *
+ * <p>This method also handles the case where group sets are distinct but
+ * there is a call to {@code GROUP_ID}. That call is replaced by the integer
+ * literal {@code 0}.
*
* <p>Also see the discussion in
* <a
href="https://issues.apache.org/jira/browse/CALCITE-1824";>[CALCITE-1824]
- * GROUP_ID returns wrong result</a>.
+ * GROUP_ID returns wrong result</a> and
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-4748";>[CALCITE-4748]
+ * If there are duplicate GROUPING SETS, Calcite should return duplicate
+ * rows</a>.
*/
- private RelBuilder rewriteAggregateWithGroupId(ImmutableBitSet groupSet,
+ private RelBuilder rewriteAggregateWithDuplicateGroupSets(
+ ImmutableBitSet groupSet,
ImmutableSortedMultiset<ImmutableBitSet> groupSets,
List<AggCall> aggregateCalls) {
final List<String> fieldNamesIfNoRewrite =
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index b8c1a52..9656adb 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -1675,7 +1675,10 @@ public class RelBuilderTest {
}
}
- @Test void testAggregateGroupingSetDuplicateIgnored() {
+ /** Tests that, if you try to create an Aggregate with duplicate grouping
+ * sets, RelBuilder creates a Union. Each branch of the Union has an
+ * Aggregate that has distinct grouping sets. */
+ @Test void testAggregateGroupingSetDuplicate() {
final RelBuilder builder = RelBuilder.create(config().build());
RelNode root =
builder.scan("EMP")
@@ -1687,8 +1690,11 @@ public class RelBuilderTest {
ImmutableBitSet.of(7))))
.build();
final String expected = ""
- + "LogicalAggregate(group=[{6, 7}], groups=[[{6}, {7}]])\n"
- + " LogicalTableScan(table=[[scott, EMP]])\n";
+ + "LogicalUnion(all=[true])\n"
+ + " LogicalAggregate(group=[{6, 7}], groups=[[{6}, {7}]])\n"
+ + " LogicalTableScan(table=[[scott, EMP]])\n"
+ + " LogicalAggregate(group=[{6, 7}], groups=[[{7}]])\n"
+ + " LogicalTableScan(table=[[scott, EMP]])\n";
assertThat(root, hasTree(expected));
}
diff --git
a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index b488522..11bad99 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -557,7 +557,27 @@ class SqlToRelConverterTest extends SqlToRelTestBase {
@Test void testGroupingSetsRepeated() {
final String sql = "select deptno, group_id()\n"
+ "from emp\n"
- + "group by grouping sets (deptno, (), deptno)";
+ + "group by grouping sets (deptno, (), job, (deptno, job), deptno,\n"
+ + " job, deptno)";
+ sql(sql).ok();
+ }
+
+ /** As {@link #testGroupingSetsRepeated()} but with no {@code GROUP_ID}
+ * function. (We still need the plan to contain a Union.) */
+ @Test void testGroupingSetsRepeatedNoGroupId() {
+ final String sql = "select deptno, job\n"
+ + "from emp\n"
+ + "group by grouping sets (deptno, (), job, (deptno, job), deptno,\n"
+ + " job, deptno)";
+ sql(sql).ok();
+ }
+
+ /** As {@link #testGroupingSetsRepeated()} but grouping sets are distinct.
+ * The {@code GROUP_ID} is replaced by 0.*/
+ @Test void testGroupingSetsWithGroupId() {
+ final String sql = "select deptno, group_id()\n"
+ + "from emp\n"
+ + "group by grouping sets (deptno, (), job)";
sql(sql).ok();
}
diff --git
a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index 1c5c977..8c12af1 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -682,7 +682,7 @@ LogicalProject(NAME=[$0])
</Resource>
<Resource name="sql">
<![CDATA[select e.deptno,
- (select * from lateral table(DEDUP(e.deptno, e.deptno)))
+ (select * from table(DEDUP(e.deptno, e.deptno)))
from emp e]]>
</Resource>
</TestCase>
@@ -1326,9 +1326,13 @@ group by sal,
<Resource name="plan">
<![CDATA[
LogicalProject(EXPR$0=[$3])
- LogicalAggregate(group=[{0, 1, 2}], groups=[[{0, 1, 2}, {0, 1}, {0, 2}]],
EXPR$0=[SUM($0)])
- LogicalProject(SAL=[$5], DEPTNO=[$7], ENAME=[$1])
- LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalUnion(all=[true])
+ LogicalAggregate(group=[{0, 1, 2}], groups=[[{0, 1, 2}, {0, 1}, {0, 2}]],
EXPR$0=[SUM($0)])
+ LogicalProject(SAL=[$5], DEPTNO=[$7], ENAME=[$1])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0, 1, 2}], groups=[[{0, 2}]], EXPR$0=[SUM($0)])
+ LogicalProject(SAL=[$5], DEPTNO=[$7], ENAME=[$1])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
@@ -2020,19 +2024,47 @@ LogicalProject(EXPR$0=[1])
<Resource name="sql">
<![CDATA[select deptno, group_id()
from emp
-group by grouping sets (deptno, (), deptno)]]>
+group by grouping sets (deptno, (), job, (deptno, job), deptno,
+ job, deptno)]]>
+ </Resource>
+ <Resource name="plan">
+ <![CDATA[
+LogicalProject(DEPTNO=[$0], EXPR$1=[$2])
+ LogicalUnion(all=[true])
+ LogicalProject(DEPTNO=[$0], JOB=[$1], EXPR$1=[0:BIGINT])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {1}, {}]])
+ LogicalProject(DEPTNO=[$7], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(DEPTNO=[$0], JOB=[$1], EXPR$1=[1:BIGINT])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0}, {1}]])
+ LogicalProject(DEPTNO=[$7], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(DEPTNO=[$0], JOB=[$1], EXPR$1=[2:BIGINT])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0}]])
+ LogicalProject(DEPTNO=[$7], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testGroupingSetsRepeatedNoGroupId">
+ <Resource name="sql">
+ <![CDATA[select deptno, job
+from emp
+group by grouping sets (deptno, (), job, (deptno, job), deptno,
+ job, deptno)]]>
</Resource>
<Resource name="plan">
<![CDATA[
LogicalUnion(all=[true])
- LogicalProject(DEPTNO=[$0], EXPR$1=[0:BIGINT])
- LogicalAggregate(group=[{0}], groups=[[{0}, {}]])
- LogicalProject(DEPTNO=[$7])
- LogicalTableScan(table=[[CATALOG, SALES, EMP]])
- LogicalProject(DEPTNO=[$0], EXPR$1=[1:BIGINT])
- LogicalAggregate(group=[{0}])
- LogicalProject(DEPTNO=[$7])
- LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {1}, {}]])
+ LogicalProject(DEPTNO=[$7], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0}, {1}]])
+ LogicalProject(DEPTNO=[$7], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0}]])
+ LogicalProject(DEPTNO=[$7], JOB=[$2])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
@@ -2059,8 +2091,27 @@ order by 2]]>
<Resource name="plan">
<![CDATA[
LogicalSort(sort0=[$1], dir0=[ASC])
- LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {1}, {}]],
EXPR$2=[SUM($2)])
- LogicalProject(DEPTNO=[$7], ENAME=[$1], SAL=[$5])
+ LogicalUnion(all=[true])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {1}, {}]],
EXPR$2=[SUM($2)])
+ LogicalProject(DEPTNO=[$7], ENAME=[$1], SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0}]], EXPR$2=[SUM($2)])
+ LogicalProject(DEPTNO=[$7], ENAME=[$1], SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testGroupingSetsWithGroupId">
+ <Resource name="sql">
+ <![CDATA[select deptno, group_id()
+from emp
+group by grouping sets (deptno, (), job)]]>
+ </Resource>
+ <Resource name="plan">
+ <![CDATA[
+LogicalProject(DEPTNO=[$0], EXPR$1=[0:BIGINT])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0}, {1}, {}]])
+ LogicalProject(DEPTNO=[$7], JOB=[$2])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
@@ -2113,9 +2164,13 @@ order by 2]]>
<Resource name="plan">
<![CDATA[
LogicalSort(sort0=[$1], dir0=[ASC])
- LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {1}, {}]],
EXPR$2=[SUM($2)])
- LogicalProject(DEPTNO=[$7], ENAME=[$1], SAL=[$5])
- LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalUnion(all=[true])
+ LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {1}, {}]],
EXPR$2=[SUM($2)])
+ LogicalProject(DEPTNO=[$7], ENAME=[$1], SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0, 1}], groups=[[{}]], EXPR$2=[SUM($2)])
+ LogicalProject(DEPTNO=[$7], ENAME=[$1], SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
diff --git a/core/src/test/resources/sql/agg.iq
b/core/src/test/resources/sql/agg.iq
index 0401e2a..c3eb0a4 100644
--- a/core/src/test/resources/sql/agg.iq
+++ b/core/src/test/resources/sql/agg.iq
@@ -1027,6 +1027,168 @@ select deptno
!ok
+# There are duplicate GROUPING SETS
+select deptno, sum(sal) as s
+from "scott".emp as t
+group by grouping sets (deptno, deptno);
++--------+----------+
+| DEPTNO | S |
++--------+----------+
+| 10 | 8750.00 |
+| 10 | 8750.00 |
+| 20 | 10875.00 |
+| 20 | 10875.00 |
+| 30 | 9400.00 |
+| 30 | 9400.00 |
++--------+----------+
+(6 rows)
+
+!ok
+
+# Similar, not duplicate GROUPING SETS
+select deptno, sum(sal) as s
+from "scott".emp as t
+group by grouping sets (deptno);
++--------+----------+
+| DEPTNO | S |
++--------+----------+
+| 10 | 8750.00 |
+| 20 | 10875.00 |
+| 30 | 9400.00 |
++--------+----------+
+(3 rows)
+
+!ok
+
+# Complex GROUPING SETS clause that contains duplicates
+select sum(sal) as s
+from "scott".emp as t
+group by job,
+ grouping sets ( deptno,
+ grouping sets ( (deptno, comm is null), comm is null),
+ (comm is null)),
+ ();
++---------+
+| S |
++---------+
+| 1300.00 |
+| 1300.00 |
+| 2450.00 |
+| 2450.00 |
+| 2850.00 |
+| 2850.00 |
+| 2975.00 |
+| 2975.00 |
+| 5000.00 |
+| 5000.00 |
+| 5000.00 |
+| 5000.00 |
+| 6000.00 |
+| 950.00 |
+| 950.00 |
+| 1900.00 |
+| 1900.00 |
+| 4150.00 |
+| 4150.00 |
+| 5600.00 |
+| 5600.00 |
+| 5600.00 |
+| 5600.00 |
+| 6000.00 |
+| 6000.00 |
+| 6000.00 |
+| 8275.00 |
+| 8275.00 |
++---------+
+(28 rows)
+
+!ok
+
+# Equivalent query using flat GROUPING SETS
+select sum(sal) as s
+from "scott".emp
+group by grouping sets ((job, deptno, comm is null),
+ (job, deptno), (job, comm is null), (job, comm is null));
++---------+
+| S |
++---------+
+| 1300.00 |
+| 1300.00 |
+| 2450.00 |
+| 2450.00 |
+| 2850.00 |
+| 2850.00 |
+| 2975.00 |
+| 2975.00 |
+| 5000.00 |
+| 5000.00 |
+| 5000.00 |
+| 5000.00 |
+| 6000.00 |
+| 950.00 |
+| 950.00 |
+| 1900.00 |
+| 1900.00 |
+| 4150.00 |
+| 4150.00 |
+| 5600.00 |
+| 5600.00 |
+| 5600.00 |
+| 5600.00 |
+| 6000.00 |
+| 6000.00 |
+| 6000.00 |
+| 8275.00 |
+| 8275.00 |
++---------+
+(28 rows)
+
+!ok
+
+# Equivalent query, but with GROUP_ID and GROUPING_ID
+select sum(sal) as s,
+ grouping_id(job, deptno, comm is null) as g,
+ group_id() as i
+from "scott".emp
+group by grouping sets ((job, deptno, comm is null),
+ (job, deptno), (job, comm is null), (job, comm is null))
+order by g, i, s desc;
++---------+---+---+
+| S | G | I |
++---------+---+---+
+| 6000.00 | 0 | 0 |
+| 5600.00 | 0 | 0 |
+| 5000.00 | 0 | 0 |
+| 2975.00 | 0 | 0 |
+| 2850.00 | 0 | 0 |
+| 2450.00 | 0 | 0 |
+| 1900.00 | 0 | 0 |
+| 1300.00 | 0 | 0 |
+| 950.00 | 0 | 0 |
+| 8275.00 | 0 | 1 |
+| 6000.00 | 0 | 1 |
+| 5600.00 | 0 | 1 |
+| 5000.00 | 0 | 1 |
+| 4150.00 | 0 | 1 |
+| 6000.00 | 1 | 0 |
+| 5600.00 | 1 | 0 |
+| 5000.00 | 1 | 0 |
+| 2975.00 | 1 | 0 |
+| 2850.00 | 1 | 0 |
+| 2450.00 | 1 | 0 |
+| 1900.00 | 1 | 0 |
+| 1300.00 | 1 | 0 |
+| 950.00 | 1 | 0 |
+| 8275.00 | 2 | 0 |
+| 6000.00 | 2 | 0 |
+| 5600.00 | 2 | 0 |
+| 5000.00 | 2 | 0 |
+| 4150.00 | 2 | 0 |
++---------+---+---+
+(28 rows)
+
+!ok
+
# [KYLIN-751] Max on negative double values is not working
# [CALCITE-735] Primitive.DOUBLE.min should be large and negative
select max(v) as x, min(v) as n
