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 7fc3e1b007fd9fd921c7929f27ac6cf3e75a15fd Author: Julian Hyde <[email protected]> AuthorDate: Thu Feb 23 12:29:02 2023 -0800 Refactor: Add RelNode.stripped RelNode.stripped method removes both HepRelVertex and RelSubset, and so saves rules from worrying about the specifics of a particular planner. --- .../main/java/org/apache/calcite/plan/hep/HepPlanner.java | 3 +++ .../apache/calcite/plan/hep/HepRelMetadataProvider.java | 3 +-- .../java/org/apache/calcite/plan/hep/HepRelVertex.java | 15 +++++++++++---- .../calcite/plan/visualizer/RuleMatchVisualizer.java | 14 +++++--------- .../java/org/apache/calcite/plan/volcano/RelSubset.java | 6 ++++++ core/src/main/java/org/apache/calcite/rel/RelNode.java | 5 +++++ .../org/apache/calcite/rel/externalize/RelDotWriter.java | 13 +------------ .../apache/calcite/rel/metadata/RelMdAllPredicates.java | 8 ++------ .../org/apache/calcite/rel/metadata/RelMdCollation.java | 2 +- .../org/apache/calcite/rel/metadata/RelMdRowCount.java | 2 +- .../java/org/apache/calcite/rel/mutable/MutableRels.java | 13 ++----------- .../calcite/rel/rules/ProjectCorrelateTransposeRule.java | 14 ++++---------- .../org/apache/calcite/rel/rules/PruneEmptyRules.java | 2 +- .../java/org/apache/calcite/sql2rel/RelDecorrelator.java | 10 +++------- 14 files changed, 46 insertions(+), 64 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java b/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java index fc2f557c0b..4b2d0e4a8a 100644 --- a/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java +++ b/core/src/main/java/org/apache/calcite/plan/hep/HepPlanner.java @@ -960,6 +960,9 @@ public class HepPlanner extends AbstractRelOptPlanner { rel.recomputeDigest(); } + if (rel instanceof HepRelVertex) { + throw new AssertionError("post-condition failed: " + rel); + } return rel; } diff --git a/core/src/main/java/org/apache/calcite/plan/hep/HepRelMetadataProvider.java b/core/src/main/java/org/apache/calcite/plan/hep/HepRelMetadataProvider.java index e243c0fe15..423d298906 100644 --- a/core/src/main/java/org/apache/calcite/plan/hep/HepRelMetadataProvider.java +++ b/core/src/main/java/org/apache/calcite/plan/hep/HepRelMetadataProvider.java @@ -58,8 +58,7 @@ class HepRelMetadataProvider implements RelMetadataProvider { if (!(rel instanceof HepRelVertex)) { return null; } - HepRelVertex vertex = (HepRelVertex) rel; - final RelNode rel2 = vertex.getCurrentRel(); + final RelNode rel2 = rel.stripped(); UnboundMetadata<M> function = requireNonNull(rel.getCluster().getMetadataProvider(), "metadataProvider") .apply(rel2.getClass(), metadataClass); diff --git a/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java b/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java index 96d5f68856..887a507dd7 100644 --- a/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java +++ b/core/src/main/java/org/apache/calcite/plan/hep/HepRelVertex.java @@ -30,6 +30,10 @@ import org.checkerframework.checker.nullness.qual.Nullable; import java.util.List; +import static com.google.common.base.Preconditions.checkArgument; + +import static java.util.Objects.requireNonNull; + /** * HepRelVertex wraps a real {@link RelNode} as a vertex in a DAG representing * the entire query expression. @@ -45,10 +49,9 @@ public class HepRelVertex extends AbstractRelNode implements DelegatingMetadataR //~ Constructors ----------------------------------------------------------- HepRelVertex(RelNode rel) { - super( - rel.getCluster(), - rel.getTraitSet()); - currentRel = rel; + super(rel.getCluster(), rel.getTraitSet()); + currentRel = requireNonNull(rel, "rel"); + checkArgument(!(rel instanceof HepRelVertex)); } //~ Methods ---------------------------------------------------------------- @@ -94,6 +97,10 @@ public class HepRelVertex extends AbstractRelNode implements DelegatingMetadataR return currentRel; } + @Override public RelNode stripped() { + return currentRel; + } + /** * Returns {@link RelNode} for metadata. */ diff --git a/core/src/main/java/org/apache/calcite/plan/visualizer/RuleMatchVisualizer.java b/core/src/main/java/org/apache/calcite/plan/visualizer/RuleMatchVisualizer.java index 7851ec1564..4b2c1828b5 100644 --- a/core/src/main/java/org/apache/calcite/plan/visualizer/RuleMatchVisualizer.java +++ b/core/src/main/java/org/apache/calcite/plan/visualizer/RuleMatchVisualizer.java @@ -54,7 +54,8 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; + +import static org.apache.calcite.util.Util.transform; /** * This is a tool to visualize the rule match process of a RelOptPlanner. @@ -159,8 +160,7 @@ public class RuleMatchVisualizer implements RelOptListener { */ private void updateInitialPlan(RelNode node) { if (node instanceof HepRelVertex) { - HepRelVertex v = (HepRelVertex) node; - updateInitialPlan(v.getCurrentRel()); + updateInitialPlan(node.stripped()); return; } this.registerRelNode(node); @@ -174,12 +174,8 @@ public class RuleMatchVisualizer implements RelOptListener { * (Workaround for HepPlanner) */ private static List<RelNode> getInputs(final RelNode node) { - return node.getInputs().stream().map(n -> { - if (n instanceof HepRelVertex) { - return ((HepRelVertex) n).getCurrentRel(); - } - return n; - }).collect(Collectors.toList()); + return transform(node.getInputs(), n -> + n instanceof HepRelVertex ? n.stripped() : n); } @Override public void relChosen(RelChosenEvent event) { 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 ab9aedfe91..2cb01a390f 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 @@ -24,6 +24,7 @@ import org.apache.calcite.plan.RelOptPlanner; import org.apache.calcite.plan.RelOptUtil; 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; @@ -235,6 +236,10 @@ public class RelSubset extends AbstractRelNode { return requireNonNull(getOriginal(), "both best and original nodes are null"); } + @Override public RelNode stripped() { + return getBestOrOriginal(); + } + @Override public RelNode copy(RelTraitSet traitSet, List<RelNode> inputs) { if (inputs.isEmpty()) { final RelTraitSet traitSet1 = traitSet.simplify(); @@ -350,6 +355,7 @@ public class RelSubset extends AbstractRelNode { * Adds expression <code>rel</code> to this subset. */ void add(RelNode rel) { + assert !(rel instanceof HepRelVertex); if (set.rels.contains(rel)) { return; } diff --git a/core/src/main/java/org/apache/calcite/rel/RelNode.java b/core/src/main/java/org/apache/calcite/rel/RelNode.java index ef3b878ce7..a97b283977 100644 --- a/core/src/main/java/org/apache/calcite/rel/RelNode.java +++ b/core/src/main/java/org/apache/calcite/rel/RelNode.java @@ -425,6 +425,11 @@ public interface RelNode extends RelOptNode, Cloneable { return getRowType().getFieldList().get(i).getType().isNullable(); } + /** Returns this node without any wrapper added by the planner. */ + default RelNode stripped() { + return this; + } + /** Context of a relational expression, for purposes of checking validity. */ interface Context { Set<CorrelationId> correlationIds(); diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelDotWriter.java b/core/src/main/java/org/apache/calcite/rel/externalize/RelDotWriter.java index 88d137b946..74d82d78ed 100644 --- a/core/src/main/java/org/apache/calcite/rel/externalize/RelDotWriter.java +++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelDotWriter.java @@ -16,8 +16,6 @@ */ package org.apache.calcite.rel.externalize; -import org.apache.calcite.plan.hep.HepRelVertex; -import org.apache.calcite.plan.volcano.RelSubset; import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.metadata.RelMetadataQuery; @@ -165,16 +163,7 @@ public class RelDotWriter extends RelWriterImpl { } private static List<RelNode> getInputs(RelNode parent) { - return Util.transform(parent.getInputs(), child -> { - if (child instanceof HepRelVertex) { - return ((HepRelVertex) child).getCurrentRel(); - } else if (child instanceof RelSubset) { - RelSubset subset = (RelSubset) child; - return subset.getBestOrOriginal(); - } else { - return child; - } - }); + return Util.transform(parent.getInputs(), RelNode::stripped); } private void explainInputs(List<? extends @Nullable RelNode> inputs) { diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java index 715a83dab3..a062ab6502 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdAllPredicates.java @@ -96,16 +96,12 @@ public class RelMdAllPredicates } public @Nullable RelOptPredicateList getAllPredicates(HepRelVertex rel, RelMetadataQuery mq) { - return mq.getAllPredicates(rel.getCurrentRel()); + return mq.getAllPredicates(rel.stripped()); } public @Nullable RelOptPredicateList getAllPredicates(RelSubset rel, RelMetadataQuery mq) { - RelNode bestOrOriginal = Util.first(rel.getBest(), rel.getOriginal()); - if (bestOrOriginal == null) { - return null; - } - return mq.getAllPredicates(bestOrOriginal); + return mq.getAllPredicates(rel.stripped()); } /** diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java index 537099d9c9..f68c54339a 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java @@ -234,7 +234,7 @@ public class RelMdCollation public @Nullable ImmutableList<RelCollation> collations(HepRelVertex rel, RelMetadataQuery mq) { - return mq.collations(rel.getCurrentRel()); + return mq.collations(rel.stripped()); } public @Nullable ImmutableList<RelCollation> collations(RelSubset rel, diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdRowCount.java b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdRowCount.java index 2066c867fe..fc1266d9a9 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdRowCount.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdRowCount.java @@ -70,7 +70,7 @@ public class RelMdRowCount @SuppressWarnings("CatchAndPrintStackTrace") public @Nullable Double getRowCount(RelSubset subset, RelMetadataQuery mq) { if (!Bug.CALCITE_1048_FIXED) { - return mq.getRowCount(subset.getBestOrOriginal()); + return mq.getRowCount(subset.stripped()); } Double v = null; for (RelNode r : subset.getRels()) { diff --git a/core/src/main/java/org/apache/calcite/rel/mutable/MutableRels.java b/core/src/main/java/org/apache/calcite/rel/mutable/MutableRels.java index 183b7955f4..54761eeef0 100644 --- a/core/src/main/java/org/apache/calcite/rel/mutable/MutableRels.java +++ b/core/src/main/java/org/apache/calcite/rel/mutable/MutableRels.java @@ -70,8 +70,6 @@ import java.util.List; import java.util.Objects; import java.util.stream.Collectors; -import static java.util.Objects.requireNonNull; - /** Utilities for dealing with {@link MutableRel}s. */ public abstract class MutableRels { @@ -329,17 +327,10 @@ public abstract class MutableRels { public static MutableRel toMutable(RelNode rel) { if (rel instanceof HepRelVertex) { - return toMutable(((HepRelVertex) rel).getCurrentRel()); + return toMutable(rel.stripped()); } if (rel instanceof RelSubset) { - RelSubset subset = (RelSubset) rel; - RelNode best = subset.getBest(); - if (best == null) { - best = - requireNonNull(subset.getOriginal(), - () -> "subset.getOriginal() is null for " + subset); - } - return toMutable(best); + return toMutable(rel.stripped()); } if (rel instanceof TableScan) { return MutableScan.of((TableScan) rel); diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ProjectCorrelateTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ProjectCorrelateTransposeRule.java index b80afeeb5a..92c0807298 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectCorrelateTransposeRule.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectCorrelateTransposeRule.java @@ -18,8 +18,6 @@ package org.apache.calcite.rel.rules; import org.apache.calcite.plan.RelOptRuleCall; import org.apache.calcite.plan.RelRule; -import org.apache.calcite.plan.hep.HepRelVertex; -import org.apache.calcite.plan.volcano.RelSubset; import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.RelShuttleImpl; import org.apache.calcite.rel.core.Correlate; @@ -195,14 +193,10 @@ public class ProjectCorrelateTransposeRule this.rexVisitor = rexVisitor; } - @Override protected RelNode visitChild(RelNode parent, int i, RelNode child) { - if (child instanceof HepRelVertex) { - child = ((HepRelVertex) child).getCurrentRel(); - } else if (child instanceof RelSubset) { - RelSubset subset = (RelSubset) child; - child = subset.getBestOrOriginal(); - } - return super.visitChild(parent, i, child).accept(rexVisitor); + @Override protected RelNode visitChild(RelNode parent, int i, + RelNode input) { + return super.visitChild(parent, i, input.stripped()) + .accept(rexVisitor); } } diff --git a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java index bd5a9e8c27..b32d6525fe 100644 --- a/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java +++ b/core/src/main/java/org/apache/calcite/rel/rules/PruneEmptyRules.java @@ -132,7 +132,7 @@ public abstract class PruneEmptyRules { return ((Values) node).getTuples().isEmpty(); } if (node instanceof HepRelVertex) { - return isEmpty(((HepRelVertex) node).getCurrentRel()); + return isEmpty(node.stripped()); } // Note: relation input might be a RelSubset, so we just iterate over the relations // in order to check if the subset is equivalent to an empty relation. diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java index f272d3e308..1a1507d217 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java @@ -1614,11 +1614,7 @@ public class RelDecorrelator implements ReflectiveVisitor { } private static RelNode stripHep(RelNode rel) { - if (rel instanceof HepRelVertex) { - HepRelVertex hepRelVertex = (HepRelVertex) rel; - rel = hepRelVertex.getCurrentRel(); - } - return rel; + return rel instanceof HepRelVertex ? rel.stripped() : rel; } //~ Inner Classes ---------------------------------------------------------- @@ -2033,7 +2029,7 @@ public class RelDecorrelator implements ReflectiveVisitor { right = filter.getInput(); assert right instanceof HepRelVertex; - right = ((HepRelVertex) right).getCurrentRel(); + right = right.stripped(); // check filter input contains no correlation if (RelOptUtil.getVariablesUsed(right).size() > 0) { @@ -2265,7 +2261,7 @@ public class RelDecorrelator implements ReflectiveVisitor { right = filter.getInput(); assert right instanceof HepRelVertex; - right = ((HepRelVertex) right).getCurrentRel(); + right = right.stripped(); // check filter input contains no correlation if (RelOptUtil.getVariablesUsed(right).size() > 0) {
