thomasrebele commented on code in PR #4505:
URL: https://github.com/apache/calcite/pull/4505#discussion_r2298725388


##########
core/src/main/java/org/apache/calcite/plan/Strong.java:
##########
@@ -265,6 +266,22 @@ public boolean isNull(RexNode node) {
         return anyNull(rexCall.getOperands());
       }
       return false;
+    case DIVIDE: {

Review Comment:
   I've understood the "Returns whether an expression is definitely null." as 
"If the expression is always null, return true; if the expression might 
evaluate to any non-null value, return false".
   
   The code that I've added here does consider a division to be null in certain 
cases: the second operand being null, or (the second operand being a non-zero 
literal and the first operand being null).
   
   I've added the handling because of `checkSimplify(div(a, nullInt), 
"null:INTEGER")` (with a non-null integer `a`) in 
RexProgramTest#testSimplifySimpleArithmetic. It might make more sense to handle 
this in `org.apache.calcite.rex.RexSimplify#simplifyDivide`.
   
   Assuming the Strong.Policy of the DIVIDE operator will be set to CUSTOM, as 
proposed in this PR: We could delete the DIVIDE block here, but then DIVIDE 
will fall-back to the default case and will never be considered null by 
Strong.isNull (even if the operands are null).



-- 
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