This is an automated email from the ASF dual-hosted git repository.
mihaibudiu 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 3b9d9f0acd [CALCITE-7511] Broaden RelShuttle Javadoc to drop "logical"
qualifier
3b9d9f0acd is described below
commit 3b9d9f0acd076410e8acfb6b9fa72ce42de41e10
Author: Venkata krishnan Sowrirajan <[email protected]>
AuthorDate: Fri Jun 5 10:21:09 2026 -0700
[CALCITE-7511] Broaden RelShuttle Javadoc to drop "logical" qualifier
---
.../apache/calcite/rel/RelHomogeneousShuttle.java | 50 +++++++++
.../java/org/apache/calcite/rel/RelShuttle.java | 29 ++++-
.../org/apache/calcite/rel/RelShuttleImpl.java | 45 ++++++++
.../java/org/apache/calcite/rel/core/Collect.java | 5 +
.../java/org/apache/calcite/rel/core/Combine.java | 5 +
.../calcite/rel/core/ConditionalCorrelate.java | 5 +
.../java/org/apache/calcite/rel/core/Sample.java | 5 +
.../java/org/apache/calcite/rel/core/Snapshot.java | 5 +
.../org/apache/calcite/rel/core/SortExchange.java | 5 +
.../apache/calcite/rel/core/TableFunctionScan.java | 5 +
.../org/apache/calcite/rel/core/TableSpool.java | 5 +
.../org/apache/calcite/rel/core/Uncollect.java | 5 +
.../java/org/apache/calcite/rel/core/Window.java | 5 +
.../calcite/rel/logical/ToLogicalConverter.java | 8 ++
.../calcite/test/RelShuttleCoverageTest.java | 118 +++++++++++++++++++++
.../apache/calcite/test/SqlHintsConverterTest.java | 35 +++---
site/_docs/history.md | 18 ++++
17 files changed, 335 insertions(+), 18 deletions(-)
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 d38f39649c..82b98a8f22 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 f9203058cb..ec735ad911 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;
@@ -36,7 +45,7 @@
import org.apache.calcite.rel.logical.LogicalValues;
/**
- * Visitor that has methods for the common logical relational expressions.
+ * Visitor that has methods for the common relational expressions.
*/
public interface RelShuttle {
RelNode visit(TableScan scan);
@@ -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 1c0d0dd5ee..377f6ab5b2 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 c308bfe6bf..475cc966d3 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 6c99a3a4de..7095a5a1c8 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<RelNode> 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 f5e1d38377..c67aaf5648 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 a4dbfacea1..f3c224ea87 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 52c4c8ab72..a7c387c663 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 af1cea599a..7320fe2768 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 0a8c6d9cfd..4e1ad50b73 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<RelNode> 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 bf22d2c962..a3483491c5 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 1fa71fc90b..09b4d5822f 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 2adf54c4f3..b444c8468f 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<RexLiteral> 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 86d08f5641..d7cee2dae7 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 0000000000..658e1bd335
--- /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<String> 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<Class<?>> visitParameters = collectVisitParameterTypes();
+ final Set<Class<? extends RelNode>> relClasses =
findConcreteRelNodesInPackages();
+
+ final Set<Class<? extends RelNode>> 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<Class<?>> 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<Class<?>> collectVisitParameterTypes() {
+ final Set<Class<?>> 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<Class<? extends RelNode>>
findConcreteRelNodesInPackages() throws IOException {
+ final Set<Class<? extends RelNode>> 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<? extends RelNode> relClass = (Class<? extends RelNode>) 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 b17f64aba6..9183134cd4 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 ac3be25990..c72d6f78d9 100644
--- a/site/_docs/history.md
+++ b/site/_docs/history.md
@@ -151,6 +151,24 @@ #### Breaking Changes
the table functions `HOP`, `TUMBLE`, `SESSION` to match the original
type of the timestamp column. These types used to be hardwired to
`TIMESTAMP(3)`.
+* [<a
href="https://issues.apache.org/jira/browse/CALCITE-7511">CALCITE-7511</a>]
+ 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}