tkalkirill commented on code in PR #5004:
URL: https://github.com/apache/calcite/pull/5004#discussion_r3414821201
##########
core/src/main/java/org/apache/calcite/rex/RexUtil.java:
##########
@@ -840,6 +840,25 @@ public static boolean isDeterministic(RexNode e) {
}
}
+ /** Returns whether an expression contains a dynamic function. */
+ public static boolean containsDynamicFunction(RexNode e) {
+ try {
+ e.accept(
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitCall(RexCall call) {
+ if (call.getOperator().isDynamicFunction()) {
+ throw Util.FoundOne.NULL;
Review Comment:
Good point. There are two identical recursive checks for dynamic parameters.
I will move this logic to `RexUtil.containsDynamicParam` and reuse it.
The remaining `instanceof RexDynamicParam` checks are intentionally local
because they distinguish a dynamic parameter as the root node from a parameter
nested inside an expression, so replacing all of them with one predicate would
change their semantics.
The remaining checks have different semantics: some detect a root dynamic
parameter, while others search recursively or decide whether an expression can
be evaluated during planning. Converging all of them would require a broader
API and changes outside the scope of CALCITE-7592. I suggest keeping those
checks explicit and considering a broader cleanup separately.
--
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]