asolimando commented on code in PR #4135:
URL: https://github.com/apache/calcite/pull/4135#discussion_r1912139312


##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -1752,27 +1752,33 @@ private static void splitJoinCondition(
    * {@code IS NOT DISTINCT FROM} function call.
    *
    * <p>For example: {@code t1.key IS NOT DISTINCT FROM t2.key}
-   * can rewritten in expanded form as
+   * can be rewritten in expanded form as
    * {@code t1.key = t2.key OR (t1.key IS NULL AND t2.key IS NULL)}.
    *
-   * @param call       Function expression to try collapsing
+   * @param rexCall       Function expression to try collapsing
    * @param rexBuilder {@link RexBuilder} instance to create new {@link 
RexCall} instances.
    * @return If the given function is an expanded IS NOT DISTINCT FROM 
function call,
-   *         return a IS NOT DISTINCT FROM function call. Otherwise return the 
input
+   *         return a IS NOT DISTINCT FROM function call. Otherwise, return 
the input
    *         function call as it is.
    */
-  public static RexCall collapseExpandedIsNotDistinctFromExpr(final RexCall 
call,
+  public static RexCall collapseExpandedIsNotDistinctFromExpr(final RexCall 
rexCall,
       final RexBuilder rexBuilder) {
-    switch (call.getKind()) {
-    case OR:
-      return doCollapseExpandedIsNotDistinctFromOrExpr(call, rexBuilder);
-
-    case CASE:
-      return doCollapseExpandedIsNotDistinctFromCaseExpr(call, rexBuilder);
-
-    default:
-      return call;
-    }
+    final RexShuttle shuttle = new RexShuttle() {
+      @Override public RexNode visitCall(RexCall call) {
+        switch (call.getKind()) {
+        case OR:
+          return doCollapseExpandedIsNotDistinctFromOrExpr(call, rexBuilder);
+        case CASE:
+          return doCollapseExpandedIsNotDistinctFromCaseExpr(call, rexBuilder);
+        case NOT:
+        case AND:
+          return super.visitCall(call);
+        default:
+          return call;

Review Comment:
   Great @linorosa, good to hear that,I think that having a fully recursive 
implementation would be useful.
   
   If you write a few more complex tests with some good mix of operators I 
think it's fine, as @mihaibudiu suggested.
   
   I skimmed the changes and they LGTM, if we manage to get this last change to 
the PR, it will be good to go.
   
   Feel free ping me whenever you feel it's ready to be reviewed again.



-- 
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: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to