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


##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,98 @@ trueLiteral, literal(1),
     checkSimplifyUnchanged(div(cast(vVarchar(), tInt(false)), nullInt));
   }
 
+  /**
+   * Test cases for <a 
href="https://issues.apache.org/jira/browse/CALCITE-7722";>[CALCITE-7722]
+   * RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and 
unsafe operands
+   * can be further simplified</a>.
+   */
+  @Test void testSimplifyIsNotNullDistributesAcrossStrongOpWithLossyCast() {
+    // "(CAST(?0.varchar0):INTEGER + 1) IS NOT NULL" ==> "IS NOT 
NULL(CAST(?0.varchar0):INTEGER)"
+    // The outer PLUS is strong AND shallow-safe; distribution keeps the
+    // non-lossless CAST inside the rewrapped IS NOT NULL
+    checkSimplify(
+        isNotNull(plus(cast(vVarchar(), tInt(true)), literal(1))),
+        "IS NOT NULL(CAST(?0.varchar0):INTEGER)");
+
+    // Symmetric IS NULL peel:
+    // "(CAST(?0.varchar0):INTEGER + 1) IS NULL" ==> "IS 
NULL(CAST(?0.varchar0):INTEGER)"
+    checkSimplify(
+        isNull(plus(cast(vVarchar(), tInt(true)), literal(1))),
+        "IS NULL(CAST(?0.varchar0):INTEGER)");
+
+    // Confirm this is consistent with same expression without CAST
+    checkSimplify(isNotNull(plus(vInt(), literal(1))), "IS NOT NULL(?0.int0)");
+    checkSimplify(isNull(plus(vInt(), literal(1))), "IS NULL(?0.int0)");
+
+    // MULTIPLY is also strong + shallow-safe
+    checkSimplify(
+        isNotNull(mul(cast(vVarchar(), tInt(true)), literal(2))),
+        "IS NOT NULL(CAST(?0.varchar0):INTEGER)");
+    checkSimplify(
+        isNull(mul(cast(vVarchar(), tInt(true)), literal(2))),
+        "IS NULL(CAST(?0.varchar0):INTEGER)");
+    checkSimplify(isNotNull(mul(vInt(), literal(2))), "IS NOT NULL(?0.int0)");
+    checkSimplify(isNull(mul(vInt(), literal(2))), "IS NULL(?0.int0)");
+
+    // PLUS of two non-lossless CAST
+    checkSimplify(
+        isNotNull(
+            plus(
+                cast(vVarchar(0), tInt(true)),
+                cast(vVarchar(1), tInt(true)))),
+        "AND(IS NOT NULL(CAST(?0.varchar0):INTEGER), IS NOT 
NULL(CAST(?0.varchar1):INTEGER))");
+    checkSimplify(
+        isNull(
+            plus(
+                cast(vVarchar(0), tInt(true)),
+                cast(vVarchar(1), tInt(true)))),
+        "OR(IS NULL(CAST(?0.varchar0):INTEGER), IS 
NULL(CAST(?0.varchar1):INTEGER))");
+
+    // Nested PLUS:
+    // "((CAST(?0.varchar0):INTEGER + 1) + 2) IS NOT NULL"
+    //   ==> "IS NOT NULL(CAST(?0.varchar0):INTEGER)"
+    checkSimplify(
+        isNotNull(
+            plus(plus(cast(vVarchar(), tInt(true)), literal(1)), literal(2))),
+        "IS NOT NULL(CAST(?0.varchar0):INTEGER)");
+
+    // Operators with checked arithmetic
+    checkSimplify(
+        isNotNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))),
+        "IS NOT NULL(CAST(?0.varchar0):INTEGER)");
+    checkSimplify(
+        isNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))),
+        "IS NULL(CAST(?0.varchar0):INTEGER)");
+    checkSimplify(
+        isNotNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))),
+        "IS NOT NULL(CAST(?0.varchar0):INTEGER)");
+    checkSimplify(
+        isNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))),
+        "IS NULL(CAST(?0.varchar0):INTEGER)");
+
+    // Arithmetic on INTERVAL
+    checkSimplify(
+        isNotNull(plus(cast(vVarchar(), tDate(true)), interval(10, 
TimeUnit.DAY))),
+        "IS NOT NULL(CAST(?0.varchar0):DATE)");
+    checkSimplify(
+        isNull(plus(cast(vVarchar(), tDate(true)), interval(1, 
TimeUnit.MONTH))),
+        "IS NULL(CAST(?0.varchar0):DATE)");
+
+    // The outer PLUS is shallow-safe, but the div(1, 0) is not, so the peel 
is therefore suppressed
+    checkSimplifyUnchanged(isNotNull(plus(div(literal(1), literal(0)), 
vIntNotNull())));
+    checkSimplifyUnchanged(isNull(plus(div(literal(1), literal(0)), 
vIntNotNull())));

Review Comment:
   The previous version of the PR stated the reason for not simplifying this: 
otherwise it would be simplified to `true` because RexCall#isAlwaysTrue() 
returns true for `IS NULL(1/0)`. This sounds like a bug in 
RexCall.isAlwaysTrue() to me. Could we make RexCall#isAlwaysTrue check for the 
safety of the IS [NOT] NULL operand? Potentially in a follow-up ticket if this 
turns out to be too complex.



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