You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jc...@apache.org on 2019/03/26 01:52:44 UTC

[calcite] branch master updated: [CALCITE-589] Extend unifyAggregates method to work with Grouping Sets

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

jcamacho 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 81143c8  [CALCITE-589] Extend unifyAggregates method to work with Grouping Sets
81143c8 is described below

commit 81143c8308b302a3f69eb14fb2dc6753a1b891bf
Author: Haisheng Yuan <h....@alibaba-inc.com>
AuthorDate: Thu Mar 14 19:21:39 2019 -0500

    [CALCITE-589] Extend unifyAggregates method to work with Grouping Sets
    
    Close apache/calcite#1106
---
 .../apache/calcite/plan/SubstitutionVisitor.java   | 29 +++++++++---------
 .../calcite/rel/mutable/MutableAggregate.java      |  3 +-
 .../apache/calcite/test/MaterializationTest.java   | 35 +++++++++++++++++++++-
 3 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
index 277a911..3f33414 100644
--- a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
+++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
@@ -48,7 +48,6 @@ import org.apache.calcite.sql.SqlAggFunction;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.tools.RelBuilder;
 import org.apache.calcite.tools.RelBuilderFactory;
-import org.apache.calcite.util.Bug;
 import org.apache.calcite.util.ControlFlowException;
 import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.Litmus;
@@ -1252,12 +1251,8 @@ public class SubstitutionVisitor {
 
   public static MutableRel unifyAggregates(MutableAggregate query,
       MutableAggregate target) {
-    if (query.getGroupType() != Aggregate.Group.SIMPLE
-        || target.getGroupType() != Aggregate.Group.SIMPLE) {
-      throw new AssertionError(Bug.CALCITE_461_FIXED);
-    }
     MutableRel result;
-    if (query.groupSet.equals(target.groupSet)) {
+    if (query.groupSets.equals(target.groupSets)) {
       // Same level of aggregation. Generate a project.
       final List<Integer> projects = new ArrayList<>();
       final int groupCount = query.groupSet.cardinality();
@@ -1272,16 +1267,20 @@ public class SubstitutionVisitor {
         projects.add(groupCount + i);
       }
       result = MutableRels.createProject(target, projects);
-    } else {
-      // Target is coarser level of aggregation. Generate an aggregate.
-      final ImmutableBitSet.Builder groupSet = ImmutableBitSet.builder();
-      final List<Integer> targetGroupList = target.groupSet.asList();
+    } else if (target.getGroupType() == Aggregate.Group.SIMPLE) {
+      // Query is coarser level of aggregation. Generate an aggregate.
+      final Map<Integer, Integer> map = new HashMap<>();
+      target.groupSet.forEach(k -> map.put(k, map.size()));
       for (int c : query.groupSet) {
-        int c2 = targetGroupList.indexOf(c);
-        if (c2 < 0) {
+        if (!map.containsKey(c)) {
           return null;
         }
-        groupSet.set(c2);
+      }
+      final ImmutableBitSet groupSet = query.groupSet.permute(map);
+      ImmutableList<ImmutableBitSet> groupSets = null;
+      if (query.getGroupType() != Aggregate.Group.SIMPLE) {
+        groupSets = ImmutableBitSet.ORDERING.immutableSortedCopy(
+            ImmutableBitSet.permute(query.groupSets, map));
       }
       final List<AggregateCall> aggregateCalls = new ArrayList<>();
       for (AggregateCall aggregateCall : query.aggCalls) {
@@ -1299,8 +1298,10 @@ public class SubstitutionVisitor {
                 aggregateCall.collation, aggregateCall.type,
                 aggregateCall.name));
       }
-      result = MutableAggregate.of(target, groupSet.build(), null,
+      result = MutableAggregate.of(target, groupSet, groupSets,
           aggregateCalls);
+    } else {
+      return null;
     }
     return MutableRels.createCastRel(result, query.rowType, true);
   }
diff --git a/core/src/main/java/org/apache/calcite/rel/mutable/MutableAggregate.java b/core/src/main/java/org/apache/calcite/rel/mutable/MutableAggregate.java
index 83ae7b9..0a743e3 100644
--- a/core/src/main/java/org/apache/calcite/rel/mutable/MutableAggregate.java
+++ b/core/src/main/java/org/apache/calcite/rel/mutable/MutableAggregate.java
@@ -64,12 +64,13 @@ public class MutableAggregate extends MutableSingleRel {
     return obj == this
         || obj instanceof MutableAggregate
         && groupSet.equals(((MutableAggregate) obj).groupSet)
+        && groupSets.equals(((MutableAggregate) obj).groupSets)
         && aggCalls.equals(((MutableAggregate) obj).aggCalls)
         && input.equals(((MutableAggregate) obj).input);
   }
 
   @Override public int hashCode() {
-    return Objects.hash(input, groupSet, aggCalls);
+    return Objects.hash(input, groupSet, groupSets, aggCalls);
   }
 
   @Override public StringBuilder digest(StringBuilder buf) {
diff --git a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
index fd767de..c99fd40 100644
--- a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
+++ b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
@@ -536,9 +536,26 @@ public class MaterializationTest {
         "select count(*) + 1 as c, \"deptno\" from \"emps\" group by \"deptno\"");
   }
 
+  /** Aggregation query at same level of aggregation as aggregation
+   * materialization with grouping sets. */
+  @Test public void testAggregateGroupSets1() {
+    checkMaterialize(
+        "select \"empid\", \"deptno\", count(*) as c, sum(\"salary\") as s from \"emps\" group by cube(\"empid\",\"deptno\")",
+        "select count(*) + 1 as c, \"deptno\" from \"emps\" group by cube(\"empid\",\"deptno\")");
+  }
+
+  /** Aggregation query with different grouping sets, should not
+   * do materialization. */
+  @Test public void testAggregateGroupSets2() {
+    checkNoMaterialize(
+        "select \"empid\", \"deptno\", count(*) as c, sum(\"salary\") as s from \"emps\" group by cube(\"empid\",\"deptno\")",
+        "select count(*) + 1 as c, \"deptno\" from \"emps\" group by rollup(\"empid\",\"deptno\")",
+        HR_FKUK_MODEL);
+  }
+
   /** Aggregation query at coarser level of aggregation than aggregation
    * materialization. Requires an additional aggregate to roll up. Note that
-   * COUNT is rolled up using SUM. */
+   * COUNT is rolled up using SUM0. */
   @Test public void testAggregateRollUp() {
     checkMaterialize(
         "select \"empid\", \"deptno\", count(*) as c, sum(\"empid\") as s from \"emps\" "
@@ -552,6 +569,22 @@ public class MaterializationTest {
                 + "    EnumerableTableScan(table=[[hr, m0]])"));
   }
 
+  /** Aggregation query with groupSets at coarser level of aggregation than
+   * aggregation materialization. Requires an additional aggregate to roll up.
+   * Note that COUNT is rolled up using SUM0. */
+  @Test public void testAggregateGroupSetsRollUp() {
+    checkMaterialize(
+        "select \"empid\", \"deptno\", count(*) as c, sum(\"salary\") as s from \"emps\" "
+            + "group by \"empid\", \"deptno\"",
+        "select count(*) + 1 as c,  \"deptno\" from \"emps\" group by cube(\"empid\",\"deptno\")",
+        HR_FKUK_MODEL,
+        CalciteAssert.checkResultContains(
+            "EnumerableCalc(expr#0..2=[{inputs}], expr#3=[1], "
+                + "expr#4=[+($t2, $t3)], C=[$t4], deptno=[$t1])\n"
+                + "  EnumerableAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {1}, {}]], agg#0=[$SUM0($2)])\n"
+                + "    EnumerableTableScan(table=[[hr, m0]])"));
+  }
+
   /** Aggregation materialization with a project. */
   @Ignore("work in progress")
   @Test public void testAggregateProject() {