[calcite] branch main updated: [CALCITE-5503] Address issue in CheapestPlanReplacer where repeated nodes in a DAG plan are not reused.

[libenchao]

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

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


The following commit(s) were added to refs/heads/main by this push:
     new 56e9deff51 [CALCITE-5503] Address issue in CheapestPlanReplacer where 
repeated nodes in a DAG plan are not reused.
56e9deff51 is described below

commit 56e9deff51942fe7673096aaf5f50544f47f1411
Author: Moritz Mack <mm...@talend.com>
AuthorDate: Mon Jan 30 11:24:40 2023 +0100

    [CALCITE-5503] Address issue in CheapestPlanReplacer where repeated nodes 
in a DAG plan are not reused.
---
 .../org/apache/calcite/plan/volcano/RelSubset.java | 10 ++++++++
 .../calcite/plan/volcano/VolcanoPlannerTest.java   | 30 ++++++++++++++++++++++
 2 files changed, 40 insertions(+)

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 06b5fc4e94..c482e6875a 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
@@ -50,6 +50,7 @@ import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -597,6 +598,7 @@ public class RelSubset extends AbstractRelNode {
    */
   static class CheapestPlanReplacer {
     VolcanoPlanner planner;
+    final Map<Integer, RelNode> visited = new HashMap<>();
 
     CheapestPlanReplacer(VolcanoPlanner planner) {
       super();
@@ -615,6 +617,13 @@ public class RelSubset extends AbstractRelNode {
         RelNode p,
         int ordinal,
         @Nullable RelNode parent) {
+      final int pId = p.getId();
+      RelNode prevVisit = visited.get(pId);
+      if (prevVisit != null) {
+        // return memoized result of previous visit if available
+        return prevVisit;
+      }
+
       if (p instanceof RelSubset) {
         RelSubset subset = (RelSubset) p;
         RelNode cheapest = subset.best;
@@ -729,6 +738,7 @@ public class RelSubset extends AbstractRelNode {
         planner.provenanceMap.put(
             p, new VolcanoPlanner.DirectProvenance(pOld));
       }
+      visited.put(pId, p); // memoize result for pId
       return p;
     }
   }
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 8aa2e1b918..9a27e3d2b0 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
@@ -139,6 +139,36 @@ class VolcanoPlannerTest {
     assertTrue(result instanceof PhysSingleRel);
   }
 
+  @Test void testMemoizeInputRelNodes() {
+    VolcanoPlanner planner = new VolcanoPlanner();
+    planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
+    RelOptCluster cluster = newCluster(planner);
+
+    // The rule that triggers the assert rule
+    planner.addRule(PhysLeafRule.INSTANCE);
+    planner.addRule(GoodSingleRule.INSTANCE);
+
+    // Leaf RelNode
+    NoneLeafRel leafRel = new NoneLeafRel(cluster, "a");
+    RelNode leafPhy = planner
+        .changeTraits(leafRel, cluster.traitSetOf(PHYS_CALLING_CONVENTION));
+
+    // RelNode with leaf RelNode as single input
+    NoneSingleRel singleRel = new NoneSingleRel(cluster, leafPhy);
+    RelNode singlePhy = planner
+        .changeTraits(singleRel, cluster.traitSetOf(PHYS_CALLING_CONVENTION));
+
+    // Binary RelNode with identical input on either side
+    PhysBiRel parent = new PhysBiRel(
+        cluster, cluster.traitSetOf(PHYS_CALLING_CONVENTION), singlePhy, 
singlePhy);
+    planner.setRoot(parent);
+
+    RelNode result = planner.chooseDelegate().findBestExp();
+
+    // Expect inputs to remain identical
+    assertEquals(result.getInput(0), result.getInput(1));
+  }
+
   @Test void testPlanToDot() {
     VolcanoPlanner planner = new VolcanoPlanner();
     planner.addRelTraitDef(ConventionTraitDef.INSTANCE);