You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2017/01/06 20:52:46 UTC

[2/5] calcite git commit: [CALCITE-1544] AggregateJoinTransposeRule fails to preserve row type (Kurt Young)

[CALCITE-1544] AggregateJoinTransposeRule fails to preserve row type (Kurt Young)

The "allColumnsInAggregate" variable is inferred by expanding the
aggregate keys with equative join keys. It's not appropriate to let
this flag to determine whether we should convert the query since the
row type will easily be unmatched.

Close apache/calcite#344


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/91102baf
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/91102baf
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/91102baf

Branch: refs/heads/master
Commit: 91102baf9ad9d45764ec190fbbeb00182be9aabe
Parents: 408285f
Author: Kurt Young <yk...@gmail.com>
Authored: Wed Jan 4 15:27:40 2017 +0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Jan 6 10:45:52 2017 -0800

----------------------------------------------------------------------
 .../rel/rules/AggregateJoinTransposeRule.java   | 57 ++++++++++----------
 .../apache/calcite/test/RelOptRulesTest.java    | 29 ++++++++++
 .../org/apache/calcite/test/RelOptRulesTest.xml | 48 +++++++++++++++++
 3 files changed, 106 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/91102baf/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java
index fa76306..3aba5bf 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateJoinTransposeRule.java
@@ -29,6 +29,7 @@ import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rel.logical.LogicalAggregate;
 import org.apache.calcite.rel.logical.LogicalJoin;
 import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rex.RexBuilder;
 import org.apache.calcite.rex.RexCall;
 import org.apache.calcite.rex.RexInputRef;
@@ -306,39 +307,39 @@ public class AggregateJoinTransposeRule extends RelOptRule {
               leftSubTotal == null ? -1 : leftSubTotal,
               rightSubTotal == null ? -1 : rightSubTotal + newLeftWidth));
     }
-  b:
-    if (allColumnsInAggregate && newAggCalls.isEmpty()) {
-      // no need to aggregate
-    } else {
-      relBuilder.project(projects);
-      if (allColumnsInAggregate) {
-        // let's see if we can convert
-        List<RexNode> projects2 = new ArrayList<>();
-        for (int key : Mappings.apply(mapping, aggregate.getGroupSet())) {
-          projects2.add(relBuilder.field(key));
-        }
-        for (AggregateCall newAggCall : newAggCalls) {
-          final SqlSplittableAggFunction splitter =
-              newAggCall.getAggregation()
-                  .unwrap(SqlSplittableAggFunction.class);
-          if (splitter != null) {
-            projects2.add(
-                splitter.singleton(rexBuilder, relBuilder.peek().getRowType(),
-                    newAggCall));
-          }
-        }
-        if (projects2.size()
-            == aggregate.getGroupSet().cardinality() + newAggCalls.size()) {
-          // We successfully converted agg calls into projects.
-          relBuilder.project(projects2);
-          break b;
+
+    relBuilder.project(projects);
+
+    boolean aggConvertedToProjects = false;
+    if (allColumnsInAggregate) {
+      // let's see if we can convert aggregate into projects
+      List<RexNode> projects2 = new ArrayList<>();
+      for (int key : Mappings.apply(mapping, aggregate.getGroupSet())) {
+        projects2.add(relBuilder.field(key));
+      }
+      for (AggregateCall newAggCall : newAggCalls) {
+        final SqlSplittableAggFunction splitter =
+            newAggCall.getAggregation().unwrap(SqlSplittableAggFunction.class);
+        if (splitter != null) {
+          final RelDataType rowType = relBuilder.peek().getRowType();
+          projects2.add(splitter.singleton(rexBuilder, rowType, newAggCall));
         }
       }
+      if (projects2.size()
+          == aggregate.getGroupSet().cardinality() + newAggCalls.size()) {
+        // We successfully converted agg calls into projects.
+        relBuilder.project(projects2);
+        aggConvertedToProjects = true;
+      }
+    }
+
+    if (!aggConvertedToProjects) {
       relBuilder.aggregate(
           relBuilder.groupKey(Mappings.apply(mapping, aggregate.getGroupSet()),
               aggregate.indicator, Mappings.apply2(mapping, aggregate.getGroupSets())),
           newAggCalls);
     }
+
     call.transformTo(relBuilder.build());
   }
 
@@ -348,8 +349,8 @@ public class AggregateJoinTransposeRule extends RelOptRule {
   private static ImmutableBitSet keyColumns(ImmutableBitSet aggregateColumns,
       ImmutableList<RexNode> predicates) {
     SortedMap<Integer, BitSet> equivalence = new TreeMap<>();
-    for (RexNode pred : predicates) {
-      populateEquivalences(equivalence, pred);
+    for (RexNode predicate : predicates) {
+      populateEquivalences(equivalence, predicate);
     }
     ImmutableBitSet keyColumns = aggregateColumns;
     for (Integer aggregateColumn : aggregateColumns) {

http://git-wip-us.apache.org/repos/asf/calcite/blob/91102baf/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index b349a7e..1e36724 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -2462,6 +2462,35 @@ public class RelOptRulesTest extends RelOptTestBase {
     checkPlanning(tester, preProgram, new HepPlanner(program), sql, true);
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-1544">[CALCITE-1544]
+   * AggregateJoinTransposeRule fails to preserve row type</a>. */
+  @Test public void testPushAggregateThroughJoin4() throws Exception {
+    final HepProgram preProgram = new HepProgramBuilder()
+        .addRuleInstance(AggregateProjectMergeRule.INSTANCE)
+        .build();
+    final HepProgram program = new HepProgramBuilder()
+        .addRuleInstance(AggregateJoinTransposeRule.EXTENDED)
+        .build();
+    final String sql = "select e.deptno\n"
+        + "from sales.emp as e join sales.dept as d on e.deptno = d.deptno\n"
+        + "group by e.deptno";
+    sql(sql).withPre(preProgram).with(program).check();
+  }
+
+  @Test public void testPushAggregateThroughJoin5() throws Exception {
+    final HepProgram preProgram = new HepProgramBuilder()
+        .addRuleInstance(AggregateProjectMergeRule.INSTANCE)
+        .build();
+    final HepProgram program = new HepProgramBuilder()
+        .addRuleInstance(AggregateJoinTransposeRule.EXTENDED)
+        .build();
+    final String sql = "select e.deptno, d.deptno\n"
+        + "from sales.emp as e join sales.dept as d on e.deptno = d.deptno\n"
+        + "group by e.deptno, d.deptno";
+    sql(sql).withPre(preProgram).with(program).check();
+  }
+
   /** SUM is the easiest aggregate function to split. */
   @Test public void testPushAggregateSumThroughJoin() throws Exception {
     final HepProgram preProgram = new HepProgramBuilder()

http://git-wip-us.apache.org/repos/asf/calcite/blob/91102baf/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index fd5fe5f..1c8cb99 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -4837,6 +4837,54 @@ LogicalAggregate(group=[{0, 9}])
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testPushAggregateThroughJoin4">
+        <Resource name="sql">
+            <![CDATA[select e.deptno
+from sales.emp as e join sales.dept as d on e.deptno = d.deptno
+group by e.deptno]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalAggregate(group=[{7}])
+  LogicalJoin(condition=[=($7, $9)], joinType=[inner])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+    LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject(DEPTNO=[$0])
+  LogicalJoin(condition=[=($0, $1)], joinType=[inner])
+    LogicalAggregate(group=[{7}])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+    LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+        </Resource>
+    </TestCase>
+    <TestCase name="testPushAggregateThroughJoin5">
+        <Resource name="sql">
+            <![CDATA[select e.deptno, d.deptno
+from sales.emp as e join sales.dept as d on e.deptno = d.deptno
+group by e.deptno, d.deptno]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalAggregate(group=[{7, 9}])
+  LogicalJoin(condition=[=($7, $9)], joinType=[inner])
+    LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+    LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject(DEPTNO=[$0], DEPTNO0=[$1])
+  LogicalJoin(condition=[=($0, $1)], joinType=[inner])
+    LogicalAggregate(group=[{7}])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+    LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+        </Resource>
+    </TestCase>
     <TestCase name="testPushAggregateSumThroughJoin">
         <Resource name="sql">
             <![CDATA[select e.job,sum(sal)