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 7d4c969  Following [CALCITE-3819] Prune parent RelNode when merging 
child RelSet with parent RelSet
7d4c969 is described below

commit 7d4c96981d9ef6fa25289af99505e5a15ba51f69
Author: Haisheng Yuan <h.y...@alibaba-inc.com>
AuthorDate: Sat Mar 28 22:47:02 2020 -0500

    Following [CALCITE-3819] Prune parent RelNode when merging child RelSet 
with parent RelSet
---
 .../apache/calcite/plan/volcano/VolcanoPlanner.java    | 18 ++++++++++--------
 .../apache/calcite/plan/volcano/VolcanoRuleCall.java   | 17 +++--------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java 
b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
index 9b7cdc9..373ec76 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
@@ -704,7 +704,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner {
       if (equivRel != null) {
         final RelSubset equivSubset = getSubset(equivRel);
         if (subset.set != equivSubset.set) {
-          merge(subset.set, equivSubset.set, !(rel instanceof RelSubset));
+          merge(equivSubset.set, subset.set);
         }
       }
       result = subset;
@@ -1249,7 +1249,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner 
{
           assert equivSubset.getTraitSet().equals(
               subset.getTraitSet());
           assert equivSubset.set != subset.set;
-          merge(equivSubset.set, subset.set, true);
+          merge(equivSubset.set, subset.set);
         }
       }
     }
@@ -1340,7 +1340,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner 
{
     return changeCount > 0;
   }
 
-  private RelSet merge(RelSet set, RelSet set2, boolean enableSwap) {
+  private RelSet merge(RelSet set, RelSet set2) {
     assert set != set2 : "pre: set != set2";
 
     // Find the root of set2's equivalence tree.
@@ -1354,8 +1354,11 @@ public class VolcanoPlanner extends 
AbstractRelOptPlanner {
     }
 
     // If necessary, swap the sets, so we're always merging the newer set
-    // into the older.
-    if (enableSwap && set.id > set2.id) {
+    // into the older or merging parent set into child set.
+    if (set2.getChildSets(this).contains(set)) {
+      // No-op
+    } else if (set.getChildSets(this).contains(set2)
+        || set.id > set2.id) {
       RelSet t = set;
       set = set2;
       set2 = t;
@@ -1493,7 +1496,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner 
{
         LOGGER.trace(
             "Register #{} {} (and merge sets, because it is a conversion)",
             rel.getId(), rel.getDigest());
-        merge(set, childSet, true);
+        merge(set, childSet);
 
         // During the mergers, the child set may have changed, and since
         // we're not registered yet, we won't have been informed. So
@@ -1597,8 +1600,7 @@ public class VolcanoPlanner extends AbstractRelOptPlanner 
{
         && (set != null)
         && (set.equivalentSet == null)) {
       LOGGER.trace("Register #{} {}, and merge sets", subset.getId(), subset);
-      boolean enableSwap = !set.getChildSets(this).contains(subset.set);
-      merge(subset.set, set, enableSwap);
+      merge(set, subset.set);
     }
     return subset;
   }
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 e02d848..a39ab06 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
@@ -124,20 +124,9 @@ public class VolcanoRuleCall extends RelOptRuleCall {
         volcanoPlanner.listener.ruleProductionSucceeded(event);
       }
 
-      for (int i = 0; i < rels.length; i++) {
-        if (rel == rels[i]) {
-          if (i == 0) {
-            return;
-          }
-          volcanoPlanner.setImportance(rels[0], 0d);
-          break;
-        }
-
-        final RelNode relCopy = rel;
-        if (rels[i].getInputs().stream().anyMatch(n -> n == relCopy)) {
-          volcanoPlanner.setImportance(rels[0], 0d);
-          break;
-        }
+      final RelNode relCopy = rel;
+      if (rels[0].getInputs().stream().anyMatch(n -> n == relCopy)) {
+        volcanoPlanner.setImportance(rels[0], 0d);
       }
 
       if (this.getRule() instanceof SubstitutionRule

Reply via email to