calcite-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jcama...@apache.org
Subject calcite git commit: [CALCITE-1400] AggregatePullUpConstantsRule might adjust aggregation function parameters indices wrongly
Date Thu, 29 Sep 2016 12:58:24 GMT
Repository: calcite
Updated Branches:
  refs/heads/master 0a1a190c1 -> 22ddc8202


[CALCITE-1400] AggregatePullUpConstantsRule might adjust aggregation function parameters indices
wrongly


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

Branch: refs/heads/master
Commit: 22ddc820204552834b98104428e3acb15e1050be
Parents: 0a1a190
Author: Jesus Camacho Rodriguez <jcamacho@apache.org>
Authored: Wed Sep 28 23:44:34 2016 +0100
Committer: Jesus Camacho Rodriguez <jcamacho@apache.org>
Committed: Thu Sep 29 13:57:24 2016 +0100

----------------------------------------------------------------------
 .../AggregateProjectPullUpConstantsRule.java    | 59 +++-----------------
 .../apache/calcite/test/RelOptRulesTest.java    | 10 ++++
 .../org/apache/calcite/test/RelOptRulesTest.xml | 24 ++++++++
 3 files changed, 43 insertions(+), 50 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/22ddc820/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java
b/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java
index bb7797e..3d05e89 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/AggregateProjectPullUpConstantsRule.java
@@ -34,7 +34,6 @@ import org.apache.calcite.tools.RelBuilder;
 import org.apache.calcite.tools.RelBuilderFactory;
 import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.Pair;
-import org.apache.calcite.util.Permutation;
 
 import com.google.common.collect.ImmutableMap;
 
@@ -151,57 +150,17 @@ public class AggregateProjectPullUpConstantsRule extends RelOptRule
{
     // reduce the group count.
     final RelBuilder relBuilder = call.builder();
     relBuilder.push(input);
-    if (map.navigableKeySet().first() == newGroupCount) {
-      // Clone aggregate calls.
-      final List<AggregateCall> newAggCalls = new ArrayList<>();
-      for (AggregateCall aggCall : aggregate.getAggCallList()) {
-        newAggCalls.add(
-            aggCall.adaptTo(input, aggCall.getArgList(), aggCall.filterArg,
-                groupCount, newGroupCount));
-      }
-      relBuilder.aggregate(
-          relBuilder.groupKey(newGroupSet, false, null),
-          newAggCalls);
-    } else {
-      // Create the mapping from old field positions to new field
-      // positions.
-      final Permutation mapping =
-          new Permutation(input.getRowType().getFieldCount());
-      mapping.identity();
-
-      // Ensure that the first positions in the mapping are for the new
-      // group columns.
-      for (int i = 0, groupOrdinal = 0, constOrdinal = newGroupCount;
-          i < groupCount;
-          ++i) {
-        if (map.containsKey(i)) {
-          mapping.set(i, constOrdinal++);
-        } else {
-          mapping.set(i, groupOrdinal++);
-        }
-      }
-
-      // Adjust aggregate calls for new field positions.
-      final List<AggregateCall> newAggCalls = new ArrayList<>();
-      for (AggregateCall aggCall : aggregate.getAggCallList()) {
-        final int argCount = aggCall.getArgList().size();
-        final List<Integer> args = new ArrayList<>(argCount);
-        for (int j = 0; j < argCount; j++) {
-          final Integer arg = aggCall.getArgList().get(j);
-          args.add(mapping.getTarget(arg));
-        }
-        final int filterArg = aggCall.filterArg < 0 ? aggCall.filterArg
-            : mapping.getTarget(aggCall.filterArg);
-        newAggCalls.add(
-            aggCall.adaptTo(relBuilder.peek(), args, filterArg, groupCount,
-                newGroupCount));
-      }
 
-      // Aggregate on projection.
-      relBuilder.aggregate(
-          relBuilder.groupKey(newGroupSet, false, null),
-              newAggCalls);
+    // Clone aggregate calls.
+    final List<AggregateCall> newAggCalls = new ArrayList<>();
+    for (AggregateCall aggCall : aggregate.getAggCallList()) {
+      newAggCalls.add(
+          aggCall.adaptTo(input, aggCall.getArgList(), aggCall.filterArg,
+              groupCount, newGroupCount));
     }
+    relBuilder.aggregate(
+        relBuilder.groupKey(newGroupSet, false, null),
+        newAggCalls);
 
     // Create a projection back again.
     List<Pair<RexNode, String>> projects = new ArrayList<>();

http://git-wip-us.apache.org/repos/asf/calcite/blob/22ddc820/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 d3a044e..4b2e073 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -1905,6 +1905,16 @@ public class RelOptRulesTest extends RelOptTestBase {
     checkPlanning(program, sql);
   }
 
+  @Test public void testAggregateProjectPullUpConstants() {
+    HepProgram program = new HepProgramBuilder()
+        .addRuleInstance(AggregateProjectPullUpConstantsRule.INSTANCE2)
+        .build();
+    final String sql = "select job, empno, sal, sum(sal) as s\n"
+        + "from emp where empno = 10\n"
+        + "group by job, empno, sal";
+    checkPlanning(program, sql);
+  }
+
   @Test public void testPushFilterWithRank() throws Exception {
     HepProgram program = new HepProgramBuilder()
         .addRuleInstance(FilterProjectTransposeRule.INSTANCE).build();

http://git-wip-us.apache.org/repos/asf/calcite/blob/22ddc820/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 7d8ec41..0ddee22 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -5421,4 +5421,28 @@ LogicalProject(COL1=[$0])
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testAggregateProjectPullUpConstants">
+        <Resource name="sql">
+            <![CDATA[select job, empno, sal, sum(sal) as s
+from emp where empno = 10
+group by job, empno, sal]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalAggregate(group=[{0, 1, 2}], S=[SUM($2)])
+  LogicalProject(JOB=[$2], EMPNO=[$0], SAL=[$5])
+    LogicalFilter(condition=[=($0, 10)])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalProject(JOB=[$0], EMPNO=[10], SAL=[$1], S=[$2])
+  LogicalAggregate(group=[{0, 2}], S=[SUM($2)])
+    LogicalProject(JOB=[$2], EMPNO=[$0], SAL=[$5])
+      LogicalFilter(condition=[=($0, 10)])
+        LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+        </Resource>
+    </TestCase>
 </Root>


Mime
View raw message