venkata91 commented on code in PR #4928:
URL: https://github.com/apache/calcite/pull/4928#discussion_r3249985153
##########
core/src/main/java/org/apache/calcite/rel/RelShuttle.java:
##########
@@ -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);
+
Review Comment:
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 —
take core abstract parents rather than `Logical*` variants. Happy to iterate on
the wording if you have a preferred framing.
On the design choice: putting the `accept(RelShuttle)` override on the
abstract parent ensures all subclasses, including `Enumerable*` and
engine-specific variants, hit the type-specific overload. Using `LogicalWindow`
etc. would only fix the dispatch gap for that one subtree and leave the same
bug for every other concrete subclass.
On timing: happy to defer to the committers on 1.42.0 vs 1.43.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
