[calcite] branch master updated: [CALCITE-3668] VolcanoPlanner does not match all the RelSubSet in matchRecursive

[hy...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new 9799c77  [CALCITE-3668] VolcanoPlanner does not match all the RelSubSet in matchRecursive
9799c77 is described below

commit 9799c777625a8fdf00594b1edad3c44ad912a488
Author: Haisheng Yuan <h....@alibaba-inc.com>
AuthorDate: Fri Jan 3 01:23:44 2020 -0600

    [CALCITE-3668] VolcanoPlanner does not match all the RelSubSet in matchRecursive
---
 .../java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java  |  7 +++++--
 .../java/org/apache/calcite/plan/volcano/PlannerTests.java     |  4 ++--
 .../org/apache/calcite/plan/volcano/VolcanoPlannerTest.java    | 10 ++++++++--
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java
index b8708d2..f2f48cc 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java
@@ -34,6 +34,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * <code>VolcanoRuleCall</code> implements the {@link RelOptRuleCall} interface
@@ -317,8 +318,10 @@ public class VolcanoRuleCall extends RelOptRuleCall {
           final RelSubset subset =
               (RelSubset) inputs.get(operand.ordinalInParent);
           if (operand.getMatchedClass() == RelSubset.class) {
-            // If the rule wants the whole subset, we just provide it
-            successors = ImmutableList.of(subset);
+            // Find all the sibling subsets that satisfy the traitSet of current subset.
+            successors = subset.set.subsets.stream()
+                .filter(s -> s.getTraitSet().satisfies(subset.getTraitSet()))
+                .collect(Collectors.toList());
           } else {
             successors = subset.getRelList();
           }
diff --git a/core/src/test/java/org/apache/calcite/plan/volcano/PlannerTests.java b/core/src/test/java/org/apache/calcite/plan/volcano/PlannerTests.java
index 94a657c..ed1c927 100644
--- a/core/src/test/java/org/apache/calcite/plan/volcano/PlannerTests.java
+++ b/core/src/test/java/org/apache/calcite/plan/volcano/PlannerTests.java
@@ -125,7 +125,7 @@ class PlannerTests {
     }
 
     @Override public RelNode copy(RelTraitSet traitSet, List<RelNode> inputs) {
-      assert traitSet.comprises(Convention.NONE);
+      assert traitSet.contains(Convention.NONE);
       return new NoneSingleRel(getCluster(), sole(inputs));
     }
   }
@@ -203,7 +203,7 @@ class PlannerTests {
     }
 
     public RelNode copy(RelTraitSet traitSet, List<RelNode> inputs) {
-      assert traitSet.comprises(PHYS_CALLING_CONVENTION);
+      assert traitSet.contains(PHYS_CALLING_CONVENTION);
       return new PhysSingleRel(getCluster(), sole(inputs));
     }
   }
diff --git a/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTest.java b/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTest.java
index 4d8dab7..a72ad79 100644
--- a/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTest.java
+++ b/core/src/test/java/org/apache/calcite/plan/volcano/VolcanoPlannerTest.java
@@ -28,6 +28,7 @@ import org.apache.calcite.plan.RelOptRuleCall;
 import org.apache.calcite.plan.RelOptUtil;
 import org.apache.calcite.plan.RelTraitSet;
 import org.apache.calcite.rel.RelCollationTraitDef;
+import org.apache.calcite.rel.RelCollations;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.convert.ConverterImpl;
 import org.apache.calcite.rel.convert.ConverterRule;
@@ -211,6 +212,7 @@ public class VolcanoPlannerTest {
   @Test public void testSubsetRule() {
     VolcanoPlanner planner = new VolcanoPlanner();
     planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
+    planner.addRelTraitDef(RelCollationTraitDef.INSTANCE);
 
     planner.addRule(new PhysLeafRule());
     planner.addRule(new GoodSingleRule());
@@ -230,14 +232,18 @@ public class VolcanoPlannerTest {
         planner.changeTraits(
             singleRel,
             cluster.traitSetOf(PHYS_CALLING_CONVENTION));
+    planner.changeTraits(leafRel,
+        cluster.traitSetOf(PHYS_CALLING_CONVENTION)
+        .plus(RelCollations.of(0)));
     planner.setRoot(convertedRel);
     RelNode result = planner.chooseDelegate().findBestExp();
     assertTrue(result instanceof PhysSingleRel);
     assertThat(sort(buf),
         equalTo(
             sort(
-                "NoneSingleRel:Subset#0.NONE",
-                "PhysSingleRel:Subset#0.PHYS")));
+                "NoneSingleRel:Subset#0.NONE.[]",
+                "PhysSingleRel:Subset#0.PHYS.[0]",
+                "PhysSingleRel:Subset#0.PHYS.[]")));
   }
 
   private static <E extends Comparable> List<E> sort(List<E> list) {