diff --git a/core/src/main/java/org/apache/calcite/rel/RelHomogeneousShuttle.java b/core/src/main/java/org/apache/calcite/rel/RelHomogeneousShuttle.java index d38f39649cfe..82b98a8f22a8 100644 --- a/core/src/main/java/org/apache/calcite/rel/RelHomogeneousShuttle.java +++ b/core/src/main/java/org/apache/calcite/rel/RelHomogeneousShuttle.java @@ -16,9 +16,19 @@ */ package org.apache.calcite.rel; +import org.apache.calcite.rel.core.Collect; +import org.apache.calcite.rel.core.Combine; +import org.apache.calcite.rel.core.ConditionalCorrelate; +import org.apache.calcite.rel.core.Sample; +import org.apache.calcite.rel.core.Snapshot; +import org.apache.calcite.rel.core.SortExchange; import org.apache.calcite.rel.core.TableFunctionScan; import org.apache.calcite.rel.core.TableScan; +import org.apache.calcite.rel.core.TableSpool; +import org.apache.calcite.rel.core.Uncollect; +import org.apache.calcite.rel.core.Window; import org.apache.calcite.rel.logical.LogicalAggregate; +import org.apache.calcite.rel.logical.LogicalAsofJoin; import org.apache.calcite.rel.logical.LogicalCalc; import org.apache.calcite.rel.logical.LogicalCorrelate; import org.apache.calcite.rel.logical.LogicalExchange; @@ -71,6 +81,10 @@ public class RelHomogeneousShuttle extends RelShuttleImpl { return visit((RelNode) join); } + @Override public RelNode visit(LogicalAsofJoin asofJoin) { + return visit((RelNode) asofJoin); + } + @Override public RelNode visit(LogicalCorrelate correlate) { return visit((RelNode) correlate); } @@ -106,4 +120,40 @@ public class RelHomogeneousShuttle extends RelShuttleImpl { @Override public RelNode visit(LogicalRepeatUnion repeatUnion) { return visit((RelNode) repeatUnion); } + + @Override public RelNode visit(Window window) { + return visit((RelNode) window); + } + + @Override public RelNode visit(Snapshot snapshot) { + return visit((RelNode) snapshot); + } + + @Override public RelNode visit(Collect collect) { + return visit((RelNode) collect); + } + + @Override public RelNode visit(Sample sample) { + return visit((RelNode) sample); + } + + @Override public RelNode visit(Uncollect uncollect) { + return visit((RelNode) uncollect); + } + + @Override public RelNode visit(Combine combine) { + return visit((RelNode) combine); + } + + @Override public RelNode visit(ConditionalCorrelate conditionalCorrelate) { + return visit((RelNode) conditionalCorrelate); + } + + @Override public RelNode visit(SortExchange sortExchange) { + return visit((RelNode) sortExchange); + } + + @Override public RelNode visit(TableSpool tableSpool) { + return visit((RelNode) tableSpool); + } } diff --git a/core/src/main/java/org/apache/calcite/rel/RelShuttle.java b/core/src/main/java/org/apache/calcite/rel/RelShuttle.java index f9203058cbda..28fec7fb36cc 100644 --- a/core/src/main/java/org/apache/calcite/rel/RelShuttle.java +++ b/core/src/main/java/org/apache/calcite/rel/RelShuttle.java @@ -16,8 +16,17 @@ */ package org.apache.calcite.rel; +import org.apache.calcite.rel.core.Collect; +import org.apache.calcite.rel.core.Combine; +import org.apache.calcite.rel.core.ConditionalCorrelate; +import org.apache.calcite.rel.core.Sample; +import org.apache.calcite.rel.core.Snapshot; +import org.apache.calcite.rel.core.SortExchange; import org.apache.calcite.rel.core.TableFunctionScan; import org.apache.calcite.rel.core.TableScan; +import org.apache.calcite.rel.core.TableSpool; +import org.apache.calcite.rel.core.Uncollect; +import org.apache.calcite.rel.core.Window; import org.apache.calcite.rel.logical.LogicalAggregate; import org.apache.calcite.rel.logical.LogicalAsofJoin; import org.apache.calcite.rel.logical.LogicalCalc; @@ -75,5 +84,23 @@ public interface RelShuttle { RelNode visit(LogicalRepeatUnion logicalRepeatUnion); + RelNode visit(Window window); + + RelNode visit(Snapshot snapshot); + + RelNode visit(Collect collect); + + RelNode visit(Sample sample); + + RelNode visit(Uncollect uncollect); + + RelNode visit(Combine combine); + + RelNode visit(ConditionalCorrelate conditionalCorrelate); + + RelNode visit(SortExchange sortExchange); + + RelNode visit(TableSpool tableSpool); + RelNode visit(RelNode other); } diff --git a/core/src/main/java/org/apache/calcite/rel/RelShuttleImpl.java b/core/src/main/java/org/apache/calcite/rel/RelShuttleImpl.java index 1c0d0dd5ee55..377f6ab5b2c7 100644 --- a/core/src/main/java/org/apache/calcite/rel/RelShuttleImpl.java +++ b/core/src/main/java/org/apache/calcite/rel/RelShuttleImpl.java @@ -17,8 +17,17 @@ package org.apache.calcite.rel; import org.apache.calcite.linq4j.Ord; +import org.apache.calcite.rel.core.Collect; +import org.apache.calcite.rel.core.Combine; +import org.apache.calcite.rel.core.ConditionalCorrelate; +import org.apache.calcite.rel.core.Sample; +import org.apache.calcite.rel.core.Snapshot; +import org.apache.calcite.rel.core.SortExchange; import org.apache.calcite.rel.core.TableFunctionScan; import org.apache.calcite.rel.core.TableScan; +import org.apache.calcite.rel.core.TableSpool; +import org.apache.calcite.rel.core.Uncollect; +import org.apache.calcite.rel.core.Window; import org.apache.calcite.rel.logical.LogicalAggregate; import org.apache.calcite.rel.logical.LogicalAsofJoin; import org.apache.calcite.rel.logical.LogicalCalc; @@ -147,6 +156,42 @@ protected RelNode visitChildren(RelNode rel) { return visitChildren(logicalRepeatUnion); } + @Override public RelNode visit(Window window) { + return visitChildren(window); + } + + @Override public RelNode visit(Snapshot snapshot) { + return visitChildren(snapshot); + } + + @Override public RelNode visit(Collect collect) { + return visitChildren(collect); + } + + @Override public RelNode visit(Sample sample) { + return visitChildren(sample); + } + + @Override public RelNode visit(Uncollect uncollect) { + return visitChildren(uncollect); + } + + @Override public RelNode visit(Combine combine) { + return visitChildren(combine); + } + + @Override public RelNode visit(ConditionalCorrelate conditionalCorrelate) { + return visitChildren(conditionalCorrelate); + } + + @Override public RelNode visit(SortExchange sortExchange) { + return visitChildren(sortExchange); + } + + @Override public RelNode visit(TableSpool tableSpool) { + return visitChildren(tableSpool); + } + @Override public RelNode visit(RelNode other) { return visitChildren(other); } diff --git a/core/src/main/java/org/apache/calcite/rel/core/Collect.java b/core/src/main/java/org/apache/calcite/rel/core/Collect.java index c308bfe6bf10..475cc966d3a1 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Collect.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Collect.java @@ -21,6 +21,7 @@ import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.rel.RelInput; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.SingleRel; import org.apache.calcite.rel.type.RelDataType; @@ -183,6 +184,10 @@ public RelNode copy(RelTraitSet traitSet, RelNode input) { return new Collect(getCluster(), traitSet, input, rowType()); } + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + @Override public RelWriter explainTerms(RelWriter pw) { return super.explainTerms(pw) .item("field", getFieldName()); diff --git a/core/src/main/java/org/apache/calcite/rel/core/Combine.java b/core/src/main/java/org/apache/calcite/rel/core/Combine.java index 6c99a3a4de1d..7095a5a1c805 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Combine.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Combine.java @@ -23,6 +23,7 @@ import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.rel.AbstractRelNode; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.metadata.RelMetadataQuery; import org.apache.calcite.rel.type.RelDataType; @@ -60,6 +61,10 @@ public Combine(RelOptCluster cluster, RelTraitSet traitSet, List inputs return new Combine(getCluster(), traitSet, inputs); } + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + @Override public void replaceInput(int ordinalInParent, RelNode rel) { // Combine has multiple inputs stored in an immutable list. // To replace an input, we need to create a new list with the replacement. diff --git a/core/src/main/java/org/apache/calcite/rel/core/ConditionalCorrelate.java b/core/src/main/java/org/apache/calcite/rel/core/ConditionalCorrelate.java index f5e1d38377d7..c67aaf564861 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/ConditionalCorrelate.java +++ b/core/src/main/java/org/apache/calcite/rel/core/ConditionalCorrelate.java @@ -19,6 +19,7 @@ import org.apache.calcite.plan.RelOptCluster; import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.hint.RelHint; import org.apache.calcite.rel.rules.CoreRules; @@ -69,6 +70,10 @@ public abstract ConditionalCorrelate copy(RelTraitSet traitSet, RelNode left, Re CorrelationId correlationId, ImmutableBitSet requiredColumns, JoinRelType joinType, RexNode condition); + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + @Override public RelWriter explainTerms(RelWriter pw) { return super.explainTerms(pw) .itemIf("condition", condition, !condition.isAlwaysTrue()); diff --git a/core/src/main/java/org/apache/calcite/rel/core/Sample.java b/core/src/main/java/org/apache/calcite/rel/core/Sample.java index a4dbfacea17e..f3c224ea87ff 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Sample.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Sample.java @@ -22,6 +22,7 @@ import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.rel.RelInput; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.SingleRel; @@ -79,6 +80,10 @@ private static RelOptSamplingParameters getSamplingParameters( return new Sample(getCluster(), sole(inputs), params); } + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + /** * Retrieve the sampling parameters for this Sample. */ diff --git a/core/src/main/java/org/apache/calcite/rel/core/Snapshot.java b/core/src/main/java/org/apache/calcite/rel/core/Snapshot.java index 52c4c8ab729b..a7c387c66375 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Snapshot.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Snapshot.java @@ -20,6 +20,7 @@ import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.rel.RelInput; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.SingleRel; import org.apache.calcite.rel.hint.Hintable; @@ -119,6 +120,10 @@ protected Snapshot( return copy(traitSet, getInput(), condition); } + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + @Override public RelWriter explainTerms(RelWriter pw) { return super.explainTerms(pw) .item("period", period); diff --git a/core/src/main/java/org/apache/calcite/rel/core/SortExchange.java b/core/src/main/java/org/apache/calcite/rel/core/SortExchange.java index af1cea599a4c..7320fe27684c 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/SortExchange.java +++ b/core/src/main/java/org/apache/calcite/rel/core/SortExchange.java @@ -24,6 +24,7 @@ import org.apache.calcite.rel.RelDistributionTraitDef; import org.apache.calcite.rel.RelInput; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import static java.util.Objects.requireNonNull; @@ -85,6 +86,10 @@ protected SortExchange(RelInput input) { public abstract SortExchange copy(RelTraitSet traitSet, RelNode newInput, RelDistribution newDistribution, RelCollation newCollation); + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + /** * Returns the array of {@link org.apache.calcite.rel.RelFieldCollation}s * asked for by the sort specification, from most significant to least diff --git a/core/src/main/java/org/apache/calcite/rel/core/TableFunctionScan.java b/core/src/main/java/org/apache/calcite/rel/core/TableFunctionScan.java index 0a8c6d9cfd19..4e1ad50b7343 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/TableFunctionScan.java +++ b/core/src/main/java/org/apache/calcite/rel/core/TableFunctionScan.java @@ -22,6 +22,7 @@ import org.apache.calcite.rel.AbstractRelNode; import org.apache.calcite.rel.RelInput; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.hint.Hintable; import org.apache.calcite.rel.hint.RelHint; @@ -140,6 +141,10 @@ protected TableFunctionScan(RelInput input) { //~ Methods ---------------------------------------------------------------- + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + @Override public final TableFunctionScan copy(RelTraitSet traitSet, List inputs) { return copy(traitSet, inputs, rexCall, elementType, getRowType(), diff --git a/core/src/main/java/org/apache/calcite/rel/core/TableSpool.java b/core/src/main/java/org/apache/calcite/rel/core/TableSpool.java index bf22d2c96260..a3483491c5d2 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/TableSpool.java +++ b/core/src/main/java/org/apache/calcite/rel/core/TableSpool.java @@ -21,6 +21,7 @@ import org.apache.calcite.plan.RelOptTable; import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import static java.util.Objects.requireNonNull; @@ -46,6 +47,10 @@ protected TableSpool(RelOptCluster cluster, RelTraitSet traitSet, return table; } + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + @Override public RelWriter explainTerms(RelWriter pw) { super.explainTerms(pw); return pw.item("table", table.getQualifiedName()); diff --git a/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java b/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java index 1fa71fc90b41..09b4d5822f07 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java @@ -21,6 +21,7 @@ import org.apache.calcite.plan.RelTraitSet; import org.apache.calcite.rel.RelInput; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.SingleRel; import org.apache.calcite.rel.type.RelDataType; @@ -112,6 +113,10 @@ public static Uncollect create( //~ Methods ---------------------------------------------------------------- + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + @Override public RelWriter explainTerms(RelWriter pw) { return super.explainTerms(pw) .itemIf("withOrdinality", withOrdinality, withOrdinality); diff --git a/core/src/main/java/org/apache/calcite/rel/core/Window.java b/core/src/main/java/org/apache/calcite/rel/core/Window.java index 2adf54c4f379..b444c8468f42 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Window.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Window.java @@ -25,6 +25,7 @@ import org.apache.calcite.rel.RelCollations; import org.apache.calcite.rel.RelFieldCollation; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.SingleRel; import org.apache.calcite.rel.hint.Hintable; @@ -210,6 +211,10 @@ public List getConstants() { return constants; } + @Override public RelNode accept(RelShuttle shuttle) { + return shuttle.visit(this); + } + @Override public @Nullable RelOptCost computeSelfCost(RelOptPlanner planner, RelMetadataQuery mq) { // Cost is proportional to the number of rows and the number of diff --git a/core/src/main/java/org/apache/calcite/rel/logical/ToLogicalConverter.java b/core/src/main/java/org/apache/calcite/rel/logical/ToLogicalConverter.java index 86d08f5641e3..d7cee2dae7ac 100644 --- a/core/src/main/java/org/apache/calcite/rel/logical/ToLogicalConverter.java +++ b/core/src/main/java/org/apache/calcite/rel/logical/ToLogicalConverter.java @@ -57,6 +57,14 @@ public ToLogicalConverter(RelBuilder relBuilder) { return LogicalTableScan.create(scan.getCluster(), scan.getTable(), scan.getHints()); } + @Override public RelNode visit(Window window) { + return visit((RelNode) window); + } + + @Override public RelNode visit(Uncollect uncollect) { + return visit((RelNode) uncollect); + } + @Override public RelNode visit(RelNode relNode) { if (relNode instanceof Aggregate) { final Aggregate agg = (Aggregate) relNode; diff --git a/core/src/test/java/org/apache/calcite/test/RelShuttleCoverageTest.java b/core/src/test/java/org/apache/calcite/test/RelShuttleCoverageTest.java new file mode 100644 index 000000000000..658e1bd33504 --- /dev/null +++ b/core/src/test/java/org/apache/calcite/test/RelShuttleCoverageTest.java @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.test; + +import org.apache.calcite.rel.AbstractRelNode; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.RelShuttle; + +import com.google.common.collect.ImmutableSet; +import com.google.common.reflect.ClassPath; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Comparator; +import java.util.HashSet; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Guards against introducing new dispatch gaps in {@link RelShuttle}. */ +class RelShuttleCoverageTest { + + private static final Set SCANNED_PACKAGES = + ImmutableSet.of("org.apache.calcite.rel.core", + "org.apache.calcite.rel.logical"); + + /** Every concrete RelNode in the scanned packages must be covered by a non-{@code visit(RelNode)} + * overload on {@link RelShuttle}. Without this, a {@code RelShuttleImpl} subclass that customizes + * the type-specific visitor would silently not be called for the rel. */ + @Test void everyRelNodeHasMatchingVisitOverload() throws IOException { + final Set> visitParameters = collectVisitParameterTypes(); + final Set> relClasses = findConcreteRelNodesInPackages(); + + final Set> uncovered = relClasses.stream() + .filter(c -> visitParameters.stream().noneMatch(vp -> vp.isAssignableFrom(c))) + .collect( + Collectors.toCollection(() -> + new TreeSet<>(Comparator.comparing(Class::getName)))); + + assertTrue(uncovered.isEmpty(), + () -> "RelNodes with no RelShuttle.visit(...) overload covering them " + + "(only the generic visit(RelNode) catch-all matches): " + uncovered); + } + + /** Every {@link RelShuttle#visit(...)} parameter type (other than {@code RelNode}) must declare + * its own {@code accept(RelShuttle)} so dispatch routes through the type-specific overload. */ + @Test void everyVisitParameterTypeDeclaresAccept() { + final Set> missingAccept = collectVisitParameterTypes().stream() + .filter(c -> !declaresAcceptRelShuttle(c)) + .collect( + Collectors.toCollection(() -> + new TreeSet<>(Comparator.comparing(Class::getName)))); + + assertTrue(missingAccept.isEmpty(), + () -> "RelShuttle.visit(X) parameter types whose X does not declare accept(RelShuttle): " + + missingAccept); + } + + /** Visit parameter types other than {@link RelNode} (the catch-all fallback). */ + private static Set> collectVisitParameterTypes() { + final Set> params = new HashSet<>(); + for (Method method : RelShuttle.class.getMethods()) { + if ("visit".equals(method.getName()) && method.getParameterCount() == 1) { + Class paramType = method.getParameterTypes()[0]; + if (paramType != RelNode.class) { + params.add(paramType); + } + } + } + return params; + } + + private static boolean declaresAcceptRelShuttle(Class clazz) { + try { + clazz.getDeclaredMethod("accept", RelShuttle.class); + return true; + } catch (NoSuchMethodException e) { + return false; + } + } + + private static Set> findConcreteRelNodesInPackages() throws IOException { + final Set> classes = new HashSet<>(); + final ClassPath classPath = ClassPath.from(RelShuttleCoverageTest.class.getClassLoader()); + for (String packageName : SCANNED_PACKAGES) { + for (ClassPath.ClassInfo info : classPath.getTopLevelClasses(packageName)) { + final Class clazz = info.load(); + if (RelNode.class.isAssignableFrom(clazz) + && !Modifier.isAbstract(clazz.getModifiers()) + && clazz != AbstractRelNode.class) { + @SuppressWarnings("unchecked") + Class relClass = (Class) clazz; + classes.add(relClass); + } + } + } + return classes; + } +} diff --git a/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java index b17f64aba671..9183134cd4e4 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java @@ -974,24 +974,25 @@ private static class HintCollector extends RelShuttleImpl { return super.visit(values); } - @Override public RelNode visit(RelNode other) { - if (other instanceof Window) { - Window window = (Window) other; - if (!window.getHints().isEmpty()) { - this.hintsCollect.add("Window:" + window.getHints()); - } - } else if (other instanceof Snapshot) { - Snapshot snapshot = (Snapshot) other; - if (!snapshot.getHints().isEmpty()) { - this.hintsCollect.add("Snapshot:" + snapshot.getHints()); - } - } else if (other instanceof TableFunctionScan) { - TableFunctionScan scan = (TableFunctionScan) other; - if (!scan.getHints().isEmpty()) { - this.hintsCollect.add("TableFunctionScan:" + scan.getHints()); - } + @Override public RelNode visit(TableFunctionScan scan) { + if (!scan.getHints().isEmpty()) { + this.hintsCollect.add("TableFunctionScan:" + scan.getHints()); + } + return super.visit(scan); + } + + @Override public RelNode visit(Window window) { + if (!window.getHints().isEmpty()) { + this.hintsCollect.add("Window:" + window.getHints()); + } + return super.visit(window); + } + + @Override public RelNode visit(Snapshot snapshot) { + if (!snapshot.getHints().isEmpty()) { + this.hintsCollect.add("Snapshot:" + snapshot.getHints()); } - return super.visit(other); + return super.visit(snapshot); } } } diff --git a/site/_docs/history.md b/site/_docs/history.md index 8a7da2a4f31e..6bd842ee3314 100644 --- a/site/_docs/history.md +++ b/site/_docs/history.md @@ -68,6 +68,25 @@ The same applies to `SqlBabelCreateTable` and `SqlUnpivot`. * [CALCITE-6942] Rename the method `decorrelateFetchOneSort` to `decorrelateSortWithRowNumber`. +* [CALCITE-7511] +This change adds new `visit(X)` overloads on `RelShuttle` for `TableFunctionScan`, +`Window`, `Snapshot`, `Collect`, `Sample`, `Uncollect`, `Combine`, `ConditionalCorrelate`, +`SortExchange`, and `TableSpool`. The corresponding rel class (or its abstract parent) +now overrides `accept(RelShuttle)` so dispatch routes through the type-specific overload +instead of `visit(RelNode other)`. Default implementations are provided in `RelShuttleImpl` +and `RelHomogeneousShuttle`. Callers that implement `RelShuttle` directly must add the +new `visit(X)` methods; the compiler will flag missing overrides. Callers that subclassed +`RelShuttleImpl` (or `RelHomogeneousShuttle`) and handled any of these types via `instanceof` +checks inside `visit(RelNode)` should migrate that logic to the matching `visit(X)` override — +those `instanceof` branches will silently stop firing for the affected types. Because the +`accept(RelShuttle)` override is placed on the abstract parent where one exists, **all** +subclasses now dispatch through the type-specific overload, including `Enumerable*` and +engine-specific variants; for example, `EnumerableTableFunctionScan` now also routes through +`visit(TableFunctionScan)`. This change also adds the previously-missing +`RelHomogeneousShuttle.visit(LogicalAsofJoin)` forwarding override; subclasses that relied +on `LogicalAsofJoin` not being routed through their `visit(RelNode)` override will now see +it routed there, matching every other rel type in the homogeneous shuttle. + #### New features {: #new-features-1-42-0}