diff --git a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java index 4fbe4c55c969..6e0f6b7e113f 100644 --- a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java +++ b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java @@ -826,6 +826,27 @@ public JdbcSort( offset, fetch); } + @Override public void replaceInput( + int ordinalInParent, + RelNode rel) { + checkCycle(this,this); + super.replaceInput(ordinalInParent,rel); + } + + private static void checkCycle(RelNode node, RelNode search) { + List inputs = node.getInputs(); + if ( inputs.isEmpty()) return; + for ( RelNode input : inputs) { + if ( input instanceof RelSubset ) { + input = ((RelSubset)input).getBestOrOriginal(); + } + if ( input == search) { + throw new RuntimeException("Detected cyclic dependency in JdbcSort " + search); + } + checkCycle(input,search); + } + } + @Override public @Nullable RelOptCost computeSelfCost(RelOptPlanner planner, RelMetadataQuery mq) { RelOptCost cost = super.computeSelfCost(planner, mq); 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 29b26adb366e..2ab0f0214266 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 @@ -25,11 +25,9 @@ import org.apache.calcite.plan.RelTrait; import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.plan.hep.HepRelVertex; -import org.apache.calcite.rel.AbstractRelNode; -import org.apache.calcite.rel.PhysicalNode; -import org.apache.calcite.rel.RelNode; -import org.apache.calcite.rel.RelWriter; +import org.apache.calcite.rel.*; import org.apache.calcite.rel.core.CorrelationId; +import org.apache.calcite.rel.core.TableScan; import org.apache.calcite.rel.externalize.RelWriterImpl; import org.apache.calcite.rel.metadata.RelMetadataQuery; import org.apache.calcite.rel.type.RelDataType; @@ -48,15 +46,7 @@ import java.io.PrintWriter; 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; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -388,7 +378,7 @@ void add(RelNode rel) { */ RelNode buildCheapestPlan(VolcanoPlanner planner) { CheapestPlanReplacer replacer = new CheapestPlanReplacer(planner); - final RelNode cheapest = replacer.visit(this, -1, null); + final RelNode cheapest = replacer.visit(this, -1); if (planner.getListener() != null) { RelOptListener.RelChosenEvent event = @@ -600,13 +590,22 @@ private boolean visitRel(RelNode p) { return "RelSubset#" + set.id + '.' + getTraitSet(); } + + static class VisitedNode { + final RelNode node; + final boolean processed; + VisitedNode(RelNode node, boolean processed) { + this.node = node; + this.processed = processed; + } + } /** * Visitor which walks over a tree of {@link RelSet}s, replacing each node * with the cheapest implementation of the expression. */ static class CheapestPlanReplacer { VolcanoPlanner planner; - final Map visited = new HashMap<>(); + final Map visited = new LinkedHashMap<>(); CheapestPlanReplacer(VolcanoPlanner planner) { super(); @@ -621,17 +620,20 @@ private static String traitDiff(RelTraitSet original, RelTraitSet desired) { .collect(Collectors.joining(", ", "[", "]")); } - public RelNode visit( - RelNode p, - int ordinal, - @Nullable RelNode parent) { + private RelNode visit(RelNode p, int ordinal) { final int pId = p.getId(); - RelNode prevVisit = visited.get(pId); + VisitedNode prevVisit = visited.get(pId); if (prevVisit != null) { // return memoized result of previous visit if available - return prevVisit; + if ( !prevVisit.processed) { + throw new RuntimeException("Cyclic relation graph detected at " + p + + ". Already processed nodes are: " + + visited.values().stream() + .map(it-> it.node.toString()) + .collect(Collectors.joining(", "))); + } + return prevVisit.node; } - if (p instanceof RelSubset) { RelSubset subset = (RelSubset) p; RelNode cheapest = subset.best; @@ -722,6 +724,7 @@ public RelNode visit( } p = cheapest; } + visited.put(pId, new VisitedNode(p,false)); // memoize that we are processing this node if (ordinal != -1) { if (planner.getListener() != null) { @@ -737,7 +740,7 @@ public RelNode visit( List inputs = new ArrayList<>(); for (int i = 0; i < oldInputs.size(); i++) { RelNode oldInput = oldInputs.get(i); - RelNode input = visit(oldInput, i, p); + RelNode input = visit(oldInput, i); inputs.add(input); } if (!inputs.equals(oldInputs)) { @@ -746,7 +749,7 @@ public RelNode visit( planner.provenanceMap.put( p, new VolcanoPlanner.DirectProvenance(pOld)); } - visited.put(pId, p); // memoize result for pId + visited.put(pId, new VisitedNode(p,true)); // memoize result for pId return p; } }