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]

Reply via email to