vlsi commented on a change in pull request #2625:
URL: https://github.com/apache/calcite/pull/2625#discussion_r760858078
##########
File path: core/src/main/java/org/apache/calcite/rel/RelNode.java
##########
@@ -410,7 +410,20 @@ default boolean isEnforcer() {
* @return A copy of this node incorporating changes made by the shuttle to
* this node's children
*/
- RelNode accept(RelShuttle shuttle);
+ default RelNode accept(RelShuttle shuttle) {
+ return accept((GenericRelVisitor<RelNode, RuntimeException>) shuttle);
+ }
+
+ /**
+ * Accepts a visit from a generic visitor.
+ *
+ * @param visitor
+ * @param <T> The return type of the visitor.
+ * @param <E> An exception that maybe thrown by the acceptance of the
visitor.
+ * @return
+ * @throws E A configurable exception type based on the visitor.
+ */
+ <T, E extends Throwable> T accept(GenericRelVisitor<T, E> visitor) throws E;
Review comment:
It seems to break backward compatibility as all rel nodes would have to
implement the new method :-/
Suppose there's `DrillRelNode extends RelNode` or even `MockRel extends
RelNode`.
How should they upgrade?
Would it be better to have a default implementation that throws?
--
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]
