Skip to content

[CALCITE-7511] Route RelShuttle dispatch through type-specific visit overloads #4928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
venkata91 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[CALCITE-7511] Route RelShuttle dispatch through type-specific visit overloads #4928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ff4a2ec
[CALCITE-7511] TableFunctionScan, Window, and Snapshot should overrid…
venkata91 May 8, 2026
154455d
[CALCITE-7511] Extend accept(RelShuttle) overrides to Collect, Sample…
venkata91 May 13, 2026
5d947f9
[CALCITE-7511] Rewrite history.md breaking-change entry: mark backwar…
venkata91 May 13, 2026
114f878
[CALCITE-7511] Rewrite history.md entry to match prose convention use…
venkata91 May 13, 2026
e2ed0c1
[CALCITE-7511] Broaden RelShuttle Javadoc to drop "logical" qualifier
venkata91 May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/RelHomogeneousShuttle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,7 +48,7 @@
* Visits all the relations in a homogeneous way: always redirects calls to
* {@code accept(RelNode)}.
*/
public class RelHomogeneousShuttle extends RelShuttleImpl {

Check warning on line 51 in core/src/main/java/org/apache/calcite/rel/RelHomogeneousShuttle.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Split this “Monster Class” into smaller and more specialized ones to reduce its dependencies on other classes from 29 to the maximum authorized 20 or less. 


See more on https://sonarcloud.io/project/issues?id=apache_calcite&issues=AZ4fuVRFXsppU-LMSwuq&open=AZ4fuVRFXsppU-LMSwuq&pullRequest=4928
@Override public RelNode visit(LogicalAggregate aggregate) {
return visit((RelNode) aggregate);
}
Expand Down Expand Up @@ -71,6 +81,10 @@
return visit((RelNode) join);
}

@Override public RelNode visit(LogicalAsofJoin asofJoin) {
return visit((RelNode) asofJoin);
}

@Override public RelNode visit(LogicalCorrelate correlate) {
return visit((RelNode) correlate);
}
Expand Down Expand Up @@ -106,4 +120,40 @@
@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);
}
}
29 changes: 28 additions & 1 deletion core/src/main/java/org/apache/calcite/rel/RelShuttle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);

Comment on lines +87 to +104
Copy link
Copy Markdown
Member

@zabetak zabetak May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here contradict a bit the Javadoc of this interface and change significantly its current purpose since the new visit methods are not logical relational expressions.

Copy link
Copy Markdown
Author

@venkata91 venkata91 May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Dropped the "logical" qualifier in the interface Javadoc (e2ed0c1) to reflect that several methods including the pre-existing visit(TableScan) / visit(TableFunctionScan) and the ones added in this PR. Happy to iterate on the wording if you have a preferred framing.

On timing: happy to defer to the committers on 1.42.0 vs 1.43.0.

Copy link
Copy Markdown
Contributor

@mihaibudiu mihaibudiu May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's wait for 1.43, assuming it happens soon.
There won't be enough time for people to test this breaking change before the new release.

RelNode visit(RelNode other);
}
45 changes: 45 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/RelShuttleImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -47,7 +56,7 @@
* {@link RelNode#copy(org.apache.calcite.plan.RelTraitSet, java.util.List)} if
* any children change.
*/
public class RelShuttleImpl implements RelShuttle {

Check warning on line 59 in core/src/main/java/org/apache/calcite/rel/RelShuttleImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Split this “Monster Class” into smaller and more specialized ones to reduce its dependencies on other classes from 29 to the maximum authorized 20 or less. 


See more on https://sonarcloud.io/project/issues?id=apache_calcite&issues=AZ4fuVT3XsppU-LMSwur&open=AZ4fuVT3XsppU-LMSwur&pullRequest=4928
protected final Deque<RelNode> stack = new ArrayDeque<>();

/**
Expand Down Expand Up @@ -147,6 +156,42 @@
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);
}
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/Collect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/Combine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/ConditionalCorrelate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/Sample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
*/
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/Snapshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/SortExchange.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/TableFunctionScan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/TableSpool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/Uncollect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/Window.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/logical/ToLogicalConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading