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 2016/01/22 08:29:22 UTC

[2/4] calcite git commit: [CALCITE-1059] Not valid to convert Aggregate on empty to empty if its GROUP BY key is empty

[CALCITE-1059] Not valid to convert Aggregate on empty to empty if its GROUP BY key is empty


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

Branch: refs/heads/master
Commit: 6201013a9a8fe79c234656ec20c5fe524d68b4d5
Parents: 920a648
Author: Julian Hyde <jh...@apache.org>
Authored: Thu Jan 14 17:42:28 2016 -0800
Committer: Julian Hyde <jh...@apache.org>
Committed: Thu Jan 21 15:03:41 2016 -0800

----------------------------------------------------------------------
 .../org/apache/calcite/rel/core/Aggregate.java  |  7 ++
 .../calcite/rel/rules/PruneEmptyRules.java      | 78 ++++++++++-------
 .../apache/calcite/test/RelOptRulesTest.java    | 88 +++++++++++++-------
 .../org/apache/calcite/test/RelOptRulesTest.xml | 34 ++++++++
 4 files changed, 143 insertions(+), 64 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/6201013a/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java b/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java
index 7b46940..4eedee7 100644
--- a/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java
+++ b/core/src/main/java/org/apache/calcite/rel/core/Aggregate.java
@@ -80,6 +80,13 @@ public abstract class Aggregate extends SingleRel {
         }
       };
 
+  public static final Predicate<Aggregate> IS_NOT_GRAND_TOTAL =
+      new Predicate<Aggregate>() {
+        public boolean apply(Aggregate input) {
+          return input.getGroupCount() > 0;
+        }
+      };
+
   //~ Instance fields --------------------------------------------------------
 
   public final boolean indicator;

http://git-wip-us.apache.org/repos/asf/calcite/blob/6201013a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
index 3fccdfa..8af045a 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java
@@ -25,11 +25,17 @@ import org.apache.calcite.rel.core.Aggregate;
 import org.apache.calcite.rel.core.Filter;
 import org.apache.calcite.rel.core.Join;
 import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.core.RelFactories;
 import org.apache.calcite.rel.core.Sort;
 import org.apache.calcite.rel.core.Values;
 import org.apache.calcite.rel.logical.LogicalUnion;
 import org.apache.calcite.rel.logical.LogicalValues;
 import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.tools.RelBuilderFactory;
+
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -71,33 +77,33 @@ public abstract class PruneEmptyRules {
           "Union") {
         public void onMatch(RelOptRuleCall call) {
           LogicalUnion union = call.rel(0);
-          final List<RelNode> childRels = call.getChildRels(union);
-          assert childRels != null;
-          final List<RelNode> newChildRels = new ArrayList<>();
-          for (RelNode childRel : childRels) {
-            if (!isEmpty(childRel)) {
-              newChildRels.add(childRel);
+          final List<RelNode> inputs = call.getChildRels(union);
+          assert inputs != null;
+          final List<RelNode> newInputs = new ArrayList<>();
+          for (RelNode input : inputs) {
+            if (!isEmpty(input)) {
+              newInputs.add(input);
             }
           }
-          assert newChildRels.size() < childRels.size()
+          assert newInputs.size() < inputs.size()
               : "planner promised us at least one Empty child";
-          RelNode newRel;
-          switch (newChildRels.size()) {
+          final RelBuilder builder = call.builder();
+          switch (newInputs.size()) {
           case 0:
-            newRel = empty(union);
+            builder.push(union).empty();
             break;
           case 1:
-            newRel =
+            builder.push(
                 RelOptUtil.createCastRel(
-                    newChildRels.get(0),
+                    newInputs.get(0),
                     union.getRowType(),
-                    true);
+                    true));
             break;
           default:
-            newRel = LogicalUnion.create(newChildRels, union.all);
+            builder.push(LogicalUnion.create(newInputs, union.all));
             break;
           }
-          call.transformTo(newRel);
+          call.transformTo(builder.build());
         }
       };
 
@@ -117,7 +123,8 @@ public abstract class PruneEmptyRules {
    * </ul>
    */
   public static final RelOptRule PROJECT_INSTANCE =
-      new RemoveEmptySingleRule(Project.class, "PruneEmptyProject");
+      new RemoveEmptySingleRule(Project.class, Predicates.<Project>alwaysTrue(),
+          RelFactories.LOGICAL_BUILDER, "PruneEmptyProject");
 
   /**
    * Rule that converts a {@link org.apache.calcite.rel.logical.LogicalFilter}
@@ -162,7 +169,7 @@ public abstract class PruneEmptyRules {
           Sort sort = call.rel(0);
           if (sort.fetch != null
               && RexLiteral.intValue(sort.fetch) == 0) {
-            call.transformTo(empty(sort));
+            call.transformTo(call.builder().push(sort).empty().build());
           }
         }
       };
@@ -174,11 +181,15 @@ public abstract class PruneEmptyRules {
    * <p>Examples:
    *
    * <ul>
-   * <li>Aggregate(Empty) becomes Empty
+   * <li>{@code Aggregate(key: [1, 3], Empty)} &rarr; {@code Empty}
+   *
+   * <li>{@code Aggregate(key: [], Empty)} is unchanged, because an aggregate
+   * without a GROUP BY key always returns 1 row, even over empty input
    * </ul>
    */
   public static final RelOptRule AGGREGATE_INSTANCE =
-      new RemoveEmptySingleRule(Aggregate.class, "PruneEmptyAggregate");
+      new RemoveEmptySingleRule(Aggregate.class, Aggregate.IS_NOT_GRAND_TOTAL,
+          RelFactories.LOGICAL_BUILDER, "PruneEmptyAggregate");
 
   /**
    * Rule that converts a {@link org.apache.calcite.rel.core.Join}
@@ -204,7 +215,7 @@ public abstract class PruneEmptyRules {
             // emp is empty
             return;
           }
-          call.transformTo(empty(join));
+          call.transformTo(call.builder().push(join).empty().build());
         }
       };
 
@@ -232,30 +243,33 @@ public abstract class PruneEmptyRules {
             // dept is empty
             return;
           }
-          call.transformTo(empty(join));
+          call.transformTo(call.builder().push(join).empty().build());
         }
       };
 
-  /** Creates a {@link org.apache.calcite.rel.core.Values} to replace
-   * {@code node}. */
-  private static Values empty(RelNode node) {
-    return LogicalValues.createEmpty(node.getCluster(), node.getRowType());
-  }
-
   /** Planner rule that converts a single-rel (e.g. project, sort, aggregate or
    * filter) on top of the empty relational expression into empty. */
-  private static class RemoveEmptySingleRule extends RelOptRule {
-    public RemoveEmptySingleRule(Class<? extends SingleRel> clazz,
+  public static class RemoveEmptySingleRule extends RelOptRule {
+    /** Creatse a simple RemoveEmptySingleRule. */
+    public <R extends SingleRel> RemoveEmptySingleRule(Class<R> clazz,
+        String description) {
+      this(clazz, Predicates.<R>alwaysTrue(), RelFactories.LOGICAL_BUILDER,
+          description);
+    }
+
+    /** Creatse a RemoveEmptySingleRule. */
+    public <R extends SingleRel> RemoveEmptySingleRule(Class<R> clazz,
+        Predicate<R> predicate, RelBuilderFactory relBuilderFactory,
         String description) {
       super(
-          operand(clazz,
+          operand(clazz, null, predicate,
               operand(Values.class, null, Values.IS_EMPTY, none())),
-          description);
+          relBuilderFactory, description);
     }
 
     public void onMatch(RelOptRuleCall call) {
       SingleRel single = call.rel(0);
-      call.transformTo(empty(single));
+      call.transformTo(call.builder().push(single).empty().build());
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/calcite/blob/6201013a/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 f59b9e7..bfdd32b 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -354,13 +354,11 @@ public class RelOptRulesTest extends RelOptTestBase {
             .addRuleInstance(filterOnJoin)
             .addGroupEnd()
             .build();
-    checkPlanning(tester,
-        preProgram,
-        new HepPlanner(program),
-        "select a.name\n"
-            + "from dept a\n"
-            + "left join dept b on b.deptno > 10\n"
-            + "right join dept c on b.deptno > 10\n");
+    final String sql = "select a.name\n"
+        + "from dept a\n"
+        + "left join dept b on b.deptno > 10\n"
+        + "right join dept c on b.deptno > 10\n";
+    checkPlanning(tester, preProgram, new HepPlanner(program), sql);
   }
 
   @Test public void testJoinProjectTranspose() {
@@ -377,13 +375,11 @@ public class RelOptRulesTest extends RelOptTestBase {
             .addRuleInstance(JoinProjectTransposeRule.LEFT_PROJECT_INCLUDE_OUTER)
             .addRuleInstance(ProjectMergeRule.INSTANCE)
             .build();
-    checkPlanning(tester,
-        preProgram,
-        new HepPlanner(program),
-        "select a.name\n"
-            + "from dept a\n"
-            + "left join dept b on b.deptno > 10\n"
-            + "right join dept c on b.deptno > 10\n");
+    final String sql = "select a.name\n"
+        + "from dept a\n"
+        + "left join dept b on b.deptno > 10\n"
+        + "right join dept c on b.deptno > 10\n";
+    checkPlanning(tester, preProgram, new HepPlanner(program), sql);
   }
 
   /** Test case for
@@ -1324,6 +1320,36 @@ public class RelOptRulesTest extends RelOptTestBase {
         "select * from emp order by deptno limit 0");
   }
 
+  @Test public void testEmptyAggregate() {
+    HepProgram preProgram = HepProgram.builder()
+        .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE)
+        .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE)
+        .build();
+    HepProgram program = new HepProgramBuilder()
+        .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE)
+        .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE)
+        .addRuleInstance(PruneEmptyRules.AGGREGATE_INSTANCE)
+        .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE)
+        .build();
+
+    final String sql = "select sum(empno) from emp where false group by deptno";
+    checkPlanning(tester, preProgram, new HepPlanner(program), sql);
+  }
+
+  @Test public void testEmptyAggregateEmptyKey() {
+    HepProgram preProgram = HepProgram.builder()
+        .addRuleInstance(ReduceExpressionsRule.FILTER_INSTANCE)
+        .addRuleInstance(PruneEmptyRules.PROJECT_INSTANCE)
+        .build();
+    HepProgram program = new HepProgramBuilder()
+        .addRuleInstance(PruneEmptyRules.AGGREGATE_INSTANCE)
+        .build();
+
+    final String sql = "select sum(empno) from emp where false";
+    final boolean unchanged = true;
+    checkPlanning(tester, preProgram, new HepPlanner(program), sql, unchanged);
+  }
+
   @Test public void testReduceCasts() throws Exception {
     HepProgram program = new HepProgramBuilder()
         .addRuleInstance(ReduceExpressionsRule.PROJECT_INSTANCE)
@@ -1475,13 +1501,12 @@ public class RelOptRulesTest extends RelOptTestBase {
     HepProgram program = HepProgram.builder()
         .addRuleInstance(AggregateFilterTransposeRule.INSTANCE)
         .build();
-    checkPlanning(tester, preProgram,
-        new HepPlanner(program),
-        "select empno, sal, deptno from ("
-            + "  select empno, sal, deptno"
-            + "  from emp"
-            + "  where sal > 5000)"
-            + "group by empno, sal, deptno");
+    final String sql = "select empno, sal, deptno from ("
+        + "  select empno, sal, deptno"
+        + "  from emp"
+        + "  where sal > 5000)"
+        + "group by empno, sal, deptno";
+    checkPlanning(tester, preProgram, new HepPlanner(program), sql);
   }
 
   @Test public void testPullFilterThroughAggregateGroupingSets()
@@ -1493,13 +1518,12 @@ public class RelOptRulesTest extends RelOptTestBase {
     HepProgram program = HepProgram.builder()
         .addRuleInstance(AggregateFilterTransposeRule.INSTANCE)
         .build();
-    checkPlanning(tester, preProgram,
-        new HepPlanner(program),
-        "select empno, sal, deptno from ("
-            + "  select empno, sal, deptno"
-            + "  from emp"
-            + "  where sal > 5000)"
-            + "group by rollup(empno, sal, deptno)");
+    final String sql = "select empno, sal, deptno from ("
+        + "  select empno, sal, deptno"
+        + "  from emp"
+        + "  where sal > 5000)"
+        + "group by rollup(empno, sal, deptno)";
+    checkPlanning(tester, preProgram, new HepPlanner(program), sql);
   }
 
   private void basePullConstantTroughAggregate() throws Exception {
@@ -1590,15 +1614,15 @@ public class RelOptRulesTest extends RelOptTestBase {
 
   public void transitiveInference(RelOptRule... extraRules) throws Exception {
     final DiffRepository diffRepos = getDiffRepos();
-    String sql = diffRepos.expand(null, "${sql}");
+    final String sql = diffRepos.expand(null, "${sql}");
 
-    HepProgram program = new HepProgramBuilder()
+    final HepProgram program = new HepProgramBuilder()
         .addRuleInstance(FilterJoinRule.DUMB_FILTER_ON_JOIN)
         .addRuleInstance(FilterJoinRule.JOIN)
         .addRuleInstance(FilterProjectTransposeRule.INSTANCE)
         .addRuleInstance(FilterSetOpTransposeRule.INSTANCE)
         .build();
-    HepPlanner planner = new HepPlanner(program);
+    final HepPlanner planner = new HepPlanner(program);
 
     final RelRoot root = tester.convertSqlToRel(sql);
     final RelNode relInitial = root.rel;
@@ -1629,7 +1653,7 @@ public class RelOptRulesTest extends RelOptTestBase {
         .addRuleInstance(JoinPushTransitivePredicatesRule.INSTANCE)
         .addRuleCollection(Arrays.asList(extraRules))
         .build();
-    HepPlanner planner2 = new HepPlanner(program2);
+    final HepPlanner planner2 = new HepPlanner(program2);
     planner.registerMetadataProviders(list);
     planner2.setRoot(relBefore);
     RelNode relAfter = planner2.findBestExp();

http://git-wip-us.apache.org/repos/asf/calcite/blob/6201013a/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 45ea111..49ef891 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -174,6 +174,40 @@ LogicalProject(EXPR$0=[1])
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testEmptyAggregate">
+        <Resource name="sql">
+            <![CDATA[select sum(empno) from emp where false group by deptno]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalProject(EXPR$0=[$1])
+  LogicalAggregate(group=[{0}], EXPR$0=[SUM($1)])
+    LogicalValues(tuples=[[]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalValues(tuples=[[]])
+]]>
+        </Resource>
+    </TestCase>
+    <TestCase name="testEmptyAggregateEmptyKey">
+        <Resource name="sql">
+            <![CDATA[select sum(empno) from emp where false]]>
+        </Resource>
+        <Resource name="planBefore">
+            <![CDATA[
+LogicalAggregate(group=[{}], EXPR$0=[SUM($0)])
+  LogicalValues(tuples=[[]])
+]]>
+        </Resource>
+        <Resource name="planAfter">
+            <![CDATA[
+LogicalAggregate(group=[{}], EXPR$0=[SUM($0)])
+  LogicalValues(tuples=[[]])
+]]>
+        </Resource>
+    </TestCase>
     <TestCase name="testPushFilterThroughOuterJoin">
         <Resource name="sql">
             <![CDATA[select 1 from sales.dept d left outer join sales.emp e on d.deptno = e.deptno where d.name = 'Charlie']]>