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 2015/10/23 21:20:27 UTC

incubator-calcite git commit: [CALCITE-926] Rules fail to match because of missing link to parent equivalence set (Maryann Xue)

Repository: incubator-calcite
Updated Branches:
  refs/heads/master 86610552e -> 2c339be18


 [CALCITE-926] Rules fail to match because of missing link to parent equivalence set (Maryann Xue)


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

Branch: refs/heads/master
Commit: 2c339be180d67853431387c26f8ed6726148f2cc
Parents: 8661055
Author: maryannxue <we...@intel.com>
Authored: Fri Oct 23 11:08:16 2015 -0700
Committer: Julian Hyde <jh...@apache.org>
Committed: Fri Oct 23 11:08:16 2015 -0700

----------------------------------------------------------------------
 .../apache/calcite/plan/volcano/RelSubset.java  |  4 +-
 .../plan/volcano/VolcanoPlannerTraitTest.java   | 94 ++++++++++++++++++--
 .../org/apache/calcite/test/LatticeTest.java    |  3 +-
 3 files changed, 89 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/2c339be1/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java b/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
index b3e6a5e..0ecc264 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
@@ -198,7 +198,7 @@ public class RelSubset extends AbstractRelNode {
     final Set<RelNode> list = new LinkedHashSet<RelNode>();
     for (RelNode parent : set.getParentRels()) {
       for (RelSubset rel : inputSubsets(parent)) {
-        if (rel.set == set && rel.getTraitSet().equals(traitSet)) {
+        if (rel.set == set && traitSet.satisfies(rel.getTraitSet())) {
           list.add(parent);
         }
       }
@@ -236,7 +236,7 @@ public class RelSubset extends AbstractRelNode {
   parentLoop:
     for (RelNode parent : set.getParentRels()) {
       for (RelSubset rel : inputSubsets(parent)) {
-        if (rel.set == set && rel.getTraitSet().equals(traitSet)) {
+        if (rel.set == set && traitSet.satisfies(rel.getTraitSet())) {
           list.add(parent);
           continue parentLoop;
         }

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/2c339be1/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java b/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java
index 66654da..e97a07c 100644
--- a/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java
+++ b/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTraitTest.java
@@ -17,6 +17,8 @@
 package org.apache.calcite.plan.volcano;
 
 import org.apache.calcite.adapter.enumerable.EnumerableConvention;
+import org.apache.calcite.adapter.enumerable.EnumerableRel;
+import org.apache.calcite.adapter.enumerable.EnumerableRelImplementor;
 import org.apache.calcite.plan.Convention;
 import org.apache.calcite.plan.ConventionTraitDef;
 import org.apache.calcite.plan.RelOptCluster;
@@ -72,6 +74,12 @@ public class VolcanoPlannerTraitTest {
   /**
    * Private alternate trait.
    */
+  private static final AltTrait ALT_EMPTY_TRAIT =
+      new AltTrait(ALT_TRAIT_DEF, "ALT_EMPTY");
+
+  /**
+   * Private alternate trait.
+   */
   private static final AltTrait ALT_TRAIT =
       new AltTrait(ALT_TRAIT_DEF, "ALT");
 
@@ -150,6 +158,38 @@ public class VolcanoPlannerTraitTest {
     assertTrue(child instanceof PhysLeafRel);
   }
 
+  @Test public void testRuleMatchAfterConversion() {
+    VolcanoPlanner planner = new VolcanoPlanner();
+
+    planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
+    planner.addRelTraitDef(ALT_TRAIT_DEF);
+
+    planner.addRule(new PhysToIteratorConverterRule());
+    planner.addRule(new PhysLeafRule());
+    planner.addRule(new IterSingleRule());
+    planner.addRule(new IterSinglePhysMergeRule());
+
+    RelOptCluster cluster = VolcanoPlannerTest.newCluster(planner);
+
+    NoneLeafRel noneLeafRel =
+        RelOptUtil.addTrait(
+            new NoneLeafRel(cluster, "noneLeafRel"), ALT_TRAIT);
+
+    NoneSingleRel noneRel =
+        RelOptUtil.addTrait(
+            new NoneSingleRel(cluster, noneLeafRel), ALT_EMPTY_TRAIT);
+
+    RelNode convertedRel =
+        planner.changeTraits(noneRel,
+            cluster.traitSetOf(EnumerableConvention.INSTANCE)
+                .replace(ALT_EMPTY_TRAIT));
+
+    planner.setRoot(convertedRel);
+    RelNode result = planner.chooseDelegate().findBestExp();
+
+    assertTrue(result instanceof IterMergedRel);
+  }
+
   @Ignore
   @Test public void testTraitPropagation() {
     VolcanoPlanner planner = new VolcanoPlanner();
@@ -251,7 +291,7 @@ public class VolcanoPlannerTraitTest {
     }
 
     public boolean satisfies(RelTrait trait) {
-      return equals(trait);
+      return trait.equals(ALT_EMPTY_TRAIT) || equals(trait);
     }
 
     public String toString() {
@@ -454,12 +494,7 @@ public class VolcanoPlannerTraitTest {
 
 
   /** A mix-in interface to extend {@link RelNode}, for testing. */
-  interface FooRel {
-    String implement(FooRelImplementor implementor);
-  }
-
-  /** An implementor for {@link FooRel}. */
-  interface FooRelImplementor {
+  interface FooRel extends EnumerableRel {
   }
 
   /** Relational expression with one input, that implements the {@link FooRel}
@@ -484,7 +519,8 @@ public class VolcanoPlannerTraitTest {
           sole(inputs));
     }
 
-    public String implement(FooRelImplementor implementor) {
+    @Override public Result implement(EnumerableRelImplementor implementor,
+        Prefer pref) {
       return null;
     }
   }
@@ -668,6 +704,48 @@ public class VolcanoPlannerTraitTest {
           sole(inputs));
     }
   }
+
+  /** Planner rule that converts an {@link IterSingleRel} on a
+   * {@link PhysToIteratorConverter} into a {@link IterMergedRel}. */
+  private static class IterSinglePhysMergeRule extends RelOptRule {
+    public IterSinglePhysMergeRule() {
+      super(
+          operand(IterSingleRel.class,
+              operand(PhysToIteratorConverter.class, any())));
+    }
+
+    @Override public void onMatch(RelOptRuleCall call) {
+      IterSingleRel singleRel = call.rel(0);
+      call.transformTo(
+          new IterMergedRel(singleRel.getCluster(),  null));
+    }
+  }
+
+  /** Relational expression with no inputs, that implements the {@link FooRel}
+   * mix-in interface. */
+  private static class IterMergedRel extends TestLeafRel implements FooRel {
+    public IterMergedRel(RelOptCluster cluster, String label) {
+      super(
+          cluster,
+          cluster.traitSetOf(EnumerableConvention.INSTANCE),
+          label);
+    }
+
+    public RelOptCost computeSelfCost(RelOptPlanner planner) {
+      return planner.getCostFactory().makeZeroCost();
+    }
+
+    public RelNode copy(RelTraitSet traitSet, List<RelNode> inputs) {
+      assert traitSet.comprises(EnumerableConvention.INSTANCE);
+      assert inputs.isEmpty();
+      return new IterMergedRel(getCluster(), this.getLabel());
+    }
+
+    @Override public Result implement(EnumerableRelImplementor implementor,
+        Prefer pref) {
+      return null;
+    }
+  }
 }
 
 // End VolcanoPlannerTraitTest.java

http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/2c339be1/core/src/test/java/org/apache/calcite/test/LatticeTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/LatticeTest.java b/core/src/test/java/org/apache/calcite/test/LatticeTest.java
index d4f522a..69eea60 100644
--- a/core/src/test/java/org/apache/calcite/test/LatticeTest.java
+++ b/core/src/test/java/org/apache/calcite/test/LatticeTest.java
@@ -528,8 +528,7 @@ public class LatticeTest {
         .enableMaterializations(true)
         .explainContains("EnumerableCalc(expr#0..1=[{inputs}], C=[$t1])\n"
             + "  EnumerableAggregate(group=[{0}], C=[COUNT($1)])\n"
-            + "    EnumerableCalc(expr#0..4=[{inputs}], proj#0..1=[{exprs}])\n"
-            + "      EnumerableTableScan(table=[[adhoc, m{27, 31}]])")
+            + "    EnumerableTableScan(table=[[adhoc, m{27, 31}]])")
         .returnsUnordered("C=4");
   }