[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/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/2c339be1 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/2c339be1 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/2c339be1 Branch: refs/heads/branch-release Commit: 2c339be180d67853431387c26f8ed6726148f2cc Parents: 8661055 Author: maryannxue <[email protected]> Authored: Fri Oct 23 11:08:16 2015 -0700 Committer: Julian Hyde <[email protected]> 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/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/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/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"); }
