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


##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,149 @@ 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: they cannot be peeled because they 
are not "safe"
+    // (they will throw at runtime in case of overflow)
+    checkSimplifyUnchanged(
+        isNotNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))));
+    checkSimplifyUnchanged(
+        isNull(checkedPlus(cast(vVarchar(), tInt(true)), literal(1))));
+    checkSimplifyUnchanged(
+        isNotNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))));
+    checkSimplifyUnchanged(
+        isNull(checkedMul(cast(vVarchar(), tInt(true)), literal(2))));
+
+    // Arithmetic on DATE and INTERVAL
+    checkSimplify(
+        isNotNull(sub(vDate(), cast(vVarchar(), tDate(true)))),
+        "AND(IS NOT NULL(?0.date0), IS NOT NULL(CAST(?0.varchar0):DATE))");
+    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))),

Review Comment:
   You have an approval - that's good enough. I have no objections, but I think 
you have raised more questions than you have answered. One is about what other 
operations need "safety" and don't have it correctly computed today, and the 
second, more important, is whether Calcite's "safety" is too strict.



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