This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 3427c2016be2f4dbb3d74c65a81dcb0099b258cb Author: Moritz Mack <[email protected]> AuthorDate: Mon Jan 30 11:24:40 2023 +0100 [CALCITE-5503] CheapestPlanReplacer should reuse repeated nodes in a DAG plan Close apache/calcite#3050 --- .../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);
