You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "rubenada (via GitHub)" <gi...@apache.org> on 2026/08/17 14:53:03 UTC
[PR] [CALCITE-7722] Simplify IS NULL / IS NOT NULL expressions by removing operations if they do not affect the nullability [calcite]
rubenada opened a new pull request, #5184:
URL: https://github.com/apache/calcite/pull/5184
<!--
Thanks for sending a pull request! Here are some critical tips for you:
1. READ THE GUIDE FIRST: https://calcite.apache.org/develop/#contributing
*For significant contributions, please discuss on the dev mailing list or Jira BEFORE coding.*
2. JIRA IS USUALLY MANDATORY: Ensure you have created an issue on the Calcite Jira:
https://issues.apache.org/jira/projects/CALCITE/issues
*Check existing issues first to avoid duplicates.*
*Note: A Jira is NOT required for typos and cosmetic changes (i.e., changes that are neither bugs nor features).*
3. TIMING: Strongly recommended to create the Jira BEFORE you start writing code (e.g., a day or so before posting a PR).
This gives others a chance to weigh in on your specification.
4. CRITICAL CONSISTENCY RULE
The following three items MUST match exactly in wording and meaning:
(A) The Jira Issue Title
(B) This Pull Request Title
(C) Your Git Commit Message
Format: [CALCITE-XXXX] <Description>
Example: [CALCITE-0000] Add IF NOT EXISTS clause to CREATE TABLE
Guidelines for a good Title:
- Illustrate using SQL keywords (in ALL-CAPS) rather than Java method names.
- Focus on the specification and user experience, not the implementation.
- Make it clear whether it is a bug or a feature.
- Use words like "should" to indicate desired behavior vs. current behavior (e.g., "Validator should not close model file").
5. REPRODUCTION: If fixing a bug, please provide a concise SQL example or test case to reproduce the issue for a faster review.
6. TESTING: Ensure `./gradlew build` passes and appropriate tests are added/updated.
-->
## Jira Link
[CALCITE-7722](https://issues.apache.org/jira/browse/CALCITE-7722)
## Changes Proposed
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR.
-->
RexSimplify.simplifyIsNotNull / simplifyIsNull currently bail out of the whole simplification when the input RexCall is not fully safe (i.e. isSafeExpression(a) == false). This is stricter than necessary for operators with Strong.Policy.ANY, where IS [NOT] NULL(f(x, y, ...)) is semantically equivalent to IS [NOT] NULL(x) OR/AND IS [NOT] NULL(y) OR/AND ... — the operator itself does not need to be evaluated to compute the result.
Example (regression for downstream projects such as Hive):
Before (≤ 1.34):
IS NOT NULL(CAST(key AS DOUBLE) + 1.0) → IS NOT NULL(CAST(key AS DOUBLE))
After (≥ 1.35):
IS NOT NULL(CAST(key AS DOUBLE) + 1.0) → (unchanged)
The rewrite is dropped because CAST(key AS DOUBLE) + 1.0 is a non-lossless cast wrapped in a +, so isSafeExpression returns false — even though + is Strong.ANY and the distribution is a valid rewrite regardless of the outer call's safety.
Proposed fix:
In the Strong.Policy.ANY branch, replace the full-tree safety requirement with a shallow safety check on the outer call. Because the branch rewraps the input as IS [NOT] NULL(operand_i) and joins the results with OR/AND, add a per-operand guard to prevent RexCall.isAlwaysTrue()/isAlwaysFalse() from silently collapsing a rewrapped IS [NOT] NULL(op) whose operand is typed non-nullable but not fully safe (which would otherwise erase a throwing subexpression such as 1 / 0).
Strong.Policy.NOT_NULL and CUSTOM continue to require full-tree safety, since those branches drop the subtree entirely.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5331962361
+1
(This overrides my earlier '-1'. Thank you to @rubenada for explaining in Jira the purpose of this change.)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3855834253
##########
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 could `@Ignore("CALCITE-7746")` this test too - or something equivalent.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3862099429
##########
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:
I understand @mihaibudiu , I'll try to work on the follow-up ticket asap.
I agree, IMO the current PR unveiled certain potential issues around the "isSafe" notion for RexSimplify, but did not introduce them, they were already there, latent; but that should not block applying the current simplification proposal, which seems a valid one.
I'll proceed with commit squash and will merge soon.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5424812502
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3802880849
##########
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:
Correct, the problem is that RexCall#isAlwaysTrue/isAlwaysFalse of an IS [NOT] NULL on an expression that is of a non-nullable type will always return a short-circuited true/false, without considering if the expression is safe, so we can hide a rutime issue; e.g. IS NOT NULL of a division by zero of type non-nullable would be collapsed to true. IMO this is a different improvement, I think this should be handled separately, as a follow-up ticket.
UPDATE: on a second thought, maybe if we include it in here we can simplify the current patch, checking....
UPDATE II: actually, fixing the RexCall isAlwaysTrue / isAlwaysFalse simplification for IS [NOT] NULL so that it considers isSafe makes the patch simpler, and enables more simplifications; so I've just applied it on the last commit. Thanks for pointing this out @thomasrebele !
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5436915173
Thanks for your feedback @mihaibudiu , @thomasrebele , @julianhyde
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5326800139
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5332030100
@thomasrebele Yes, we could use better terminology. "Safe", "Strict" and "Strong" are parallel concepts, dealing with exceptions (non-termination), whether arguments get evaluated, and null values.
It's necessary to distinguish an operator's propagation characteristics (e.g. whether it returns null if and only if both its arguments are null) from the characteristics of an expression (e.g. whether it may return null, or may throw).
Even if you are able to find a good terminology that the community agrees on, implementing it is a challenge - you would have to modify the source code, potentially renaming classes, and deal with the fact that there are closed issues and commits that cannot be retrospectively changed.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3855879359
##########
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:
yes, that is cleaner, I'll push a commit doing that.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3814816236
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
I hope we can merge 7728 today, so let's wait a bit more.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3835951513
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1200,15 +1200,21 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime errors
Review Comment:
Could be. IMO it talks about the general NOT_NULL case behavior, so it doesn't seem out of place currently.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3841329099
##########
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:
I agree with handling this in a follow-up ticket.
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1631,12 +1654,31 @@ enum SafeRexVisitor implements RexVisitor<Boolean> {
|| RexUtil.isLosslessCast(call)
|| safeOps.contains(sqlKind)
|| safeOperators.contains(sqlOperator)) {
- return RexVisitorImpl.visitArrayAnd(this, call.operands);
+ return !deep || RexVisitorImpl.visitArrayAnd(this, call.operands);
}
return false;
}
+ /**
+ * Shallow variant of the visitor: reports whether the OUTER node's
+ * operator can be evaluated on non-null operands without throwing at
+ * runtime. Unlike {@link #visitCall(RexCall)}, it does not recurse into
+ * the operands. Callers that only need to know whether the outer
+ * operator itself is safe (e.g. RexSimplify's {@code Strong.ANY}
+ * distribution branches, which preserve subtree evaluation) can use
+ * this in place of the full-tree {@link RexSimplify#isSafeExpression}.
+ *
+ * <p>Non-{@link RexCall} nodes are always shallow-safe (they cannot
+ * throw at their own level).
+ */
+ boolean isShallowSafe(RexNode node) {
Review Comment:
An alternative might be `isOuterSafe`.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3835921691
##########
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
Review Comment:
It means the `IS_NOT_NULL(expr)` is "distributed" among the operands of the expression: `IS_NOT_NULL(operand0) AND .... IS_NOT_NULL(operandN)`
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3811067534
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
@mihaibudiu since [CALCITE-7725](https://issues.apache.org/jira/browse/CALCITE-7725) might be more tricky than anticipated (and it seems it require other changes, e.g. [CALCITE-7728](https://issues.apache.org/jira/browse/CALCITE-7728) ), shall we move on with this PR, which seems in a good shape? (and then eventually a few tests introduced here would be adjusted when those other tickets get done).
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3802661608
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1197,15 +1197,40 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime
+ // errors inside
+ if (!isSafe) {
+ return simplifiedResult;
+ }
return rexBuilder.makeLiteral(true);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NOT NULL"
- // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL"
+ // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL".
Review Comment:
comment simplified
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1197,15 +1197,40 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime
+ // errors inside
+ if (!isSafe) {
+ return simplifiedResult;
+ }
return rexBuilder.makeLiteral(true);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NOT NULL"
- // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL"
+ // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL".
+ // This branch PRESERVES the operand subtrees (each is either recursively
+ // simplified, or rewrapped verbatim as IS NOT NULL(operand)), so it only
+ // needs SHALLOW safety of the outer operator. Requiring full-tree safety
+ // here suppresses common peels like (CAST(str):DOUBLE + 1.0) IS NOT NULL
+ // → CAST(str):DOUBLE IS NOT NULL just because a non-lossless CAST lives
+ // deeper in the tree, even though that CAST would still be evaluated
+ // inside the rewrapped IS NOT NULL(operand).
+ if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) {
+ return simplifiedResult;
+ }
+ // The peel only preserves runtime semantics when each operand can be
Review Comment:
comment simplified
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3803174445
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
The semantics has changed, hasn't it? [CHECKED_PLUS](https://github.com/apache/calcite/blob/ef667b041bb7953c91b601982b5af2b4a75b3e7a/core/src/main/java/org/apache/calcite/sql/SqlKind.java#L326) throws an exception on overflow.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada merged PR #5184:
URL: https://github.com/apache/calcite/pull/5184
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5345617683
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3832367164
##########
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(
Review Comment:
@thomasrebele @mihaibudiu I have rebased with the latest main branch (which includes the patch "CALCITE-7725 Review safety of checked arithmetic operators").
As expected, I had to adjust the tests here, since the checked arithmetic expressions are no longer simplified (because they are not considered safe, i.e. it is now working as expected).
When you have a bit of time, could you please take another look at the PR?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3803174445
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
The semantics has changed, hasn't it? [CHECKED_PLUS](https://github.com/apache/calcite/blob/ef667b041bb7953c91b601982b5af2b4a75b3e7a/core/src/main/java/org/apache/calcite/sql/SqlKind.java#L326) thrown an exception on overflow.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3804092917
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
Thank you! For the record: [CALCITE-7725](https://issues.apache.org/jira/browse/CALCITE-7725).
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3835945483
##########
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:
This seems to be consistent with how these operations are defined atm.
Is arithmetic on date/interval operands supposed to behave different compared to other operand types?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3835916443
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1631,12 +1654,31 @@ enum SafeRexVisitor implements RexVisitor<Boolean> {
|| RexUtil.isLosslessCast(call)
|| safeOps.contains(sqlKind)
|| safeOperators.contains(sqlOperator)) {
- return RexVisitorImpl.visitArrayAnd(this, call.operands);
+ return !deep || RexVisitorImpl.visitArrayAnd(this, call.operands);
}
return false;
}
+ /**
+ * Shallow variant of the visitor: reports whether the OUTER node's
+ * operator can be evaluated on non-null operands without throwing at
+ * runtime. Unlike {@link #visitCall(RexCall)}, it does not recurse into
+ * the operands. Callers that only need to know whether the outer
+ * operator itself is safe (e.g. RexSimplify's {@code Strong.ANY}
+ * distribution branches, which preserve subtree evaluation) can use
+ * this in place of the full-tree {@link RexSimplify#isSafeExpression}.
+ *
+ * <p>Non-{@link RexCall} nodes are always shallow-safe (they cannot
+ * throw at their own level).
+ */
+ boolean isShallowSafe(RexNode node) {
Review Comment:
I'd prefer to leave the current name, since the method theoretically could accept any type of RexNode, and also to avoid confusion with SqlOperator#isSafeOperator
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3835924139
##########
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
Review Comment:
It means the CAST of the tests above (which contains similar IS_NOT_NULL on PLUS expression, but with CAST operand)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3836673078
##########
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:
Yes, arithmetic on dates and intervals should not wrap around. For integers you can argue that wrap around may make sense (e.g., that's the Java semantics), but for dates or intervals it produces no meaningful results. Perhaps this is another bug -- an omission in ConvertToChecked?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3855110266
##########
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:
If the outcome of this test will change due to the other issue, maybe we should merge the other bug fix first.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3855771801
##########
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:
I have created https://issues.apache.org/jira/browse/CALCITE-7746 as a follow-up , but probably won't be able to work on that soon. I'd rather add a comment here mentioning that these tests might need an adjustment due to that ticket, and finalize this PR (to avoid blocking it more time). wdyt?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5373226816
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [82.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3803374385
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
I'll create a ticket.
I'd prefer to keep them, with a clarifying comment stating that this simplification is "controversial" and should be reviewed. So that when the follow-up ticket gets addressed, these tests would (should) fail and they would need to be adapted. Otherwise, if we remove them, we can risk that they never get reinstated back 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3802880849
##########
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:
Correct, the problem is that RexCall#isAlwaysTrue/isAlwaysFalse of an IS [NOT] NULL on an expression that is of a non-nullable type will always return a short-circuited true/false, without considering if the expression is safe, so we can hide a rutime issue; e.g. IS NOT NULL of a division by zero of type non-nullable would be collapsed to true. IMO this is a different improvement, I think this should be handled separately, as a follow-up ticket.
UPDATE: on a second thought, maybe if we include it in here we can simplify the current patch, checking....
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3803342615
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
Indeed, maybe remove the corresponding assertions and create a follow-up ticket?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] Simplify IS NULL / IS NOT NULL expressions by removing operations if they do not affect the nullability [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5317244113
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [89.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5326211469
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [89.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3803374385
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
I'll create a ticket.
I'd prefer to keep them, with a clarifying comment stating that this simplification is "controversial" and should be reviewed (mentioning the follow-up ticket). So that when that ticket gets addressed, these tests would (should) fail and they would need to be adapted. Otherwise, if we remove them, we can risk that they never get reinstated back 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3803374385
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
I'd prefer to keep them, with a clarifying comment stating that this simplification is "controversial" and should be reviewed. So that when the follow-up ticket gets addressed, these tests would (should) fail and they would need to be adapted. Otherwise, if we remove them, we can risk that they never get reinstated back 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5437458874
Thank you for working on the simplification, @rubenada!
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5424307874
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3834515084
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1200,15 +1200,21 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime errors
Review Comment:
should the comment be inside the braces?
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1631,12 +1654,31 @@ enum SafeRexVisitor implements RexVisitor<Boolean> {
|| RexUtil.isLosslessCast(call)
|| safeOps.contains(sqlKind)
|| safeOperators.contains(sqlOperator)) {
- return RexVisitorImpl.visitArrayAnd(this, call.operands);
+ return !deep || RexVisitorImpl.visitArrayAnd(this, call.operands);
}
return false;
}
+ /**
+ * Shallow variant of the visitor: reports whether the OUTER node's
+ * operator can be evaluated on non-null operands without throwing at
+ * runtime. Unlike {@link #visitCall(RexCall)}, it does not recurse into
+ * the operands. Callers that only need to know whether the outer
+ * operator itself is safe (e.g. RexSimplify's {@code Strong.ANY}
+ * distribution branches, which preserve subtree evaluation) can use
+ * this in place of the full-tree {@link RexSimplify#isSafeExpression}.
+ *
+ * <p>Non-{@link RexCall} nodes are always shallow-safe (they cannot
+ * throw at their own level).
+ */
+ boolean isShallowSafe(RexNode node) {
Review Comment:
Can this be called isOperatorSafe? After all, it's a property of the operator.
##########
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
Review Comment:
what is "distribution"?
##########
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
Review Comment:
I don't know which cast this is talking about
##########
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:
Isn't this wrong? Arithmetic on dates should be checked.
Or maybe this kind of code can never be generated?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3836881636
##########
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:
Yes, in that case I'd consider that a separate issue, to be handled in a follow-up ticket
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3851939886
##########
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:
@mihaibudiu do you have any other remarks that may block this PR? Shall I create a separate PR for the "arithmetic on dates and intervals safety review" and proceed to merge this one?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3802666450
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3024,82 @@ trueLiteral, literal(1),
checkSimplifyUnchanged(div(cast(vVarchar(), tInt(false)), nullInt));
}
+ /** Test cases for peeling {@code IS [NOT] NULL} across a strong outer
+ * operator whose subtree contains a non-lossless {@code CAST}.
+ *
+ * <p>Distributing {@code IS [NOT] NULL} across a strong operator (e.g.
+ * {@code +}, {@code *}) preserves subtree evaluation: each operand is
+ * either recursively simplified or rewrapped verbatim as
+ * {@code IS [NOT] NULL(operand)}, so the presence of a non-lossless
+ * {@code CAST} deeper in the tree must not block the distribution. */
+ @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)");
+
+ // Nested PLUS on both sides: distribution still peels one layer
+ // and stops at the inner CAST, which is not shallow-safe.
+ // "((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)");
+ }
+
+ /** The distribution must still be suppressed when the outer node is
+ * itself not shallow-safe (e.g. DIVIDE by a literal zero) or when an
+ * operand is typed non-nullable yet not fully safe, otherwise
+ * {@link org.apache.calcite.rex.RexCall#isAlwaysTrue()} would collapse
+ * the rewrapped {@code IS NOT NULL(operand)} to {@code TRUE} and hide
+ * the runtime throw. */
+ @Test void testSimplifyIsNotNullDoesNotDistributeAcrossUnsafeOuter() {
+ // The outer PLUS is shallow-safe, but the div(1, 0) operand is typed
+ // non-nullable, so the peel would rewrap it as IS NOT NULL(/(1, 0))
+ // which the trivial isAlwaysTrue() shortcut would collapse to TRUE
+ // and lose the throw. The peel is therefore suppressed.
+ checkSimplifyUnchanged(isNotNull(plus(div(literal(1), literal(0)), vIntNotNull())));
+ checkSimplifyUnchanged(isNull(plus(div(literal(1), literal(0)), vIntNotNull())));
+
+ // IS NOT NULL(x/0) itself is not peeled, because DIVIDE with a
+ // literal-zero divisor is not shallow-safe (this branch would
+ // otherwise drop the throwing subexpression).
+ checkSimplifyUnchanged(isNotNull(div(vIntNotNull(), literal(0))));
+ checkSimplifyUnchanged(isNull(div(vIntNotNull(), literal(0))));
+ checkSimplifyUnchanged(isNull(div(cast(vIntNotNull(), tBigInt()), literal(0))));
+
+ // IS NULL(CAST(10/0 AS BIGINT)) stays as IS NULL(10/0)
+ // after the lossless-CAST strip; the DIVIDE is not
+ // shallow-safe, so no further distribution occurs.
+ checkSimplify(isNull(cast(div(vIntNotNull(), literal(0)), tBigInt())),
Review Comment:
good point; added both cases
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3803235559
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
But checked operators are defined as [safeOps in RexSimplify's SafeRexVisitor](https://github.com/apache/calcite/blob/ef667b041bb7953c91b601982b5af2b4a75b3e7a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java#L1555); so simplifyIs[Not]Null is just applying the same logic. If this is incorrect, I guess it would need to be fixed 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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3806142712
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
If we need to merge the other PR first, we should.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] Simplify IS NULL / IS NOT NULL expressions by removing operations if they do not affect the nullability [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3797490503
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1197,15 +1197,40 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime
+ // errors inside
+ if (!isSafe) {
+ return simplifiedResult;
+ }
return rexBuilder.makeLiteral(true);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NOT NULL"
- // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL"
+ // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL".
Review Comment:
this comment could be shorter
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1197,15 +1197,40 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime
+ // errors inside
+ if (!isSafe) {
+ return simplifiedResult;
+ }
return rexBuilder.makeLiteral(true);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NOT NULL"
- // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL"
+ // simplifies to "operand0 IS NOT NULL AND operand1 IS NOT NULL".
+ // This branch PRESERVES the operand subtrees (each is either recursively
+ // simplified, or rewrapped verbatim as IS NOT NULL(operand)), so it only
+ // needs SHALLOW safety of the outer operator. Requiring full-tree safety
+ // here suppresses common peels like (CAST(str):DOUBLE + 1.0) IS NOT NULL
+ // → CAST(str):DOUBLE IS NOT NULL just because a non-lossless CAST lives
+ // deeper in the tree, even though that CAST would still be evaluated
+ // inside the rewrapped IS NOT NULL(operand).
+ if (!SafeRexVisitor.INSTANCE.isShallowSafe(a)) {
+ return simplifiedResult;
+ }
+ // The peel only preserves runtime semantics when each operand can be
Review Comment:
and so can this one
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3024,82 @@ trueLiteral, literal(1),
checkSimplifyUnchanged(div(cast(vVarchar(), tInt(false)), nullInt));
}
+ /** Test cases for peeling {@code IS [NOT] NULL} across a strong outer
Review Comment:
Maybe you can follow the established pattern for test case JavaDoc, "Test case for [CALCITE-7722]" etc?
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1258,15 +1286,32 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime
+ // errors inside
+ if (!isSafe) {
+ return simplifiedResult;
+ }
return rexBuilder.makeLiteral(false);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NULL" simplifies
- // to "operand0 IS NULL OR operand1 IS NULL"
+ // to "operand0 IS NULL OR operand1 IS NULL". This branch PRESERVES the
Review Comment:
And I think this one too
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3024,82 @@ trueLiteral, literal(1),
checkSimplifyUnchanged(div(cast(vVarchar(), tInt(false)), nullInt));
}
+ /** Test cases for peeling {@code IS [NOT] NULL} across a strong outer
+ * operator whose subtree contains a non-lossless {@code CAST}.
+ *
+ * <p>Distributing {@code IS [NOT] NULL} across a strong operator (e.g.
+ * {@code +}, {@code *}) preserves subtree evaluation: each operand is
+ * either recursively simplified or rewrapped verbatim as
+ * {@code IS [NOT] NULL(operand)}, so the presence of a non-lossless
+ * {@code CAST} deeper in the tree must not block the distribution. */
+ @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)");
+
+ // Nested PLUS on both sides: distribution still peels one layer
+ // and stops at the inner CAST, which is not shallow-safe.
+ // "((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)");
+ }
+
+ /** The distribution must still be suppressed when the outer node is
+ * itself not shallow-safe (e.g. DIVIDE by a literal zero) or when an
+ * operand is typed non-nullable yet not fully safe, otherwise
+ * {@link org.apache.calcite.rex.RexCall#isAlwaysTrue()} would collapse
+ * the rewrapped {@code IS NOT NULL(operand)} to {@code TRUE} and hide
+ * the runtime throw. */
+ @Test void testSimplifyIsNotNullDoesNotDistributeAcrossUnsafeOuter() {
+ // The outer PLUS is shallow-safe, but the div(1, 0) operand is typed
+ // non-nullable, so the peel would rewrap it as IS NOT NULL(/(1, 0))
+ // which the trivial isAlwaysTrue() shortcut would collapse to TRUE
+ // and lose the throw. The peel is therefore suppressed.
+ checkSimplifyUnchanged(isNotNull(plus(div(literal(1), literal(0)), vIntNotNull())));
+ checkSimplifyUnchanged(isNull(plus(div(literal(1), literal(0)), vIntNotNull())));
+
+ // IS NOT NULL(x/0) itself is not peeled, because DIVIDE with a
+ // literal-zero divisor is not shallow-safe (this branch would
+ // otherwise drop the throwing subexpression).
+ checkSimplifyUnchanged(isNotNull(div(vIntNotNull(), literal(0))));
+ checkSimplifyUnchanged(isNull(div(vIntNotNull(), literal(0))));
+ checkSimplifyUnchanged(isNull(div(cast(vIntNotNull(), tBigInt()), literal(0))));
+
+ // IS NULL(CAST(10/0 AS BIGINT)) stays as IS NULL(10/0)
+ // after the lossless-CAST strip; the DIVIDE is not
+ // shallow-safe, so no further distribution occurs.
+ checkSimplify(isNull(cast(div(vIntNotNull(), literal(0)), tBigInt())),
Review Comment:
can we have some tests using checked arithmetic or arithmetic on intervals?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3802663214
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1258,15 +1286,32 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (hasCustomNullabilityRules(a.getKind())) {
return simplifiedResult;
}
- if (!isSafe) {
- return simplifiedResult;
- }
switch (Strong.policy(a)) {
case NOT_NULL:
+ // Drops the subtree; require full-tree safety so we don't hide runtime
+ // errors inside
+ if (!isSafe) {
+ return simplifiedResult;
+ }
return rexBuilder.makeLiteral(false);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NULL" simplifies
- // to "operand0 IS NULL OR operand1 IS NULL"
+ // to "operand0 IS NULL OR operand1 IS NULL". This branch PRESERVES the
Review Comment:
comment simplified
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3024,82 @@ trueLiteral, literal(1),
checkSimplifyUnchanged(div(cast(vVarchar(), tInt(false)), nullInt));
}
+ /** Test cases for peeling {@code IS [NOT] NULL} across a strong outer
Review Comment:
Done
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3802880849
##########
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:
Correct, the problem is that RexCall#isAlwaysTrue/isAlwaysFalse of an IS [NOT] NULL on an expression that is of a non-nullable type will always return a short-circuited true/false, without considering if the expression is safe, so we can hide a rutime issue; e.g. IS NOT NULL of a division by zero of type non-nullable would be collapsed to true. IMO this is a different improvement, I think this should be handled separately, as a follow-up ticket
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5328314015
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL over a Strong.ANY call is no longer simplified when the call itself is unsafe [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5318318029
@julianhyde the patch is actually using Strong. I have update the Jira to make it more accurate with the actual fix proposal
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #5184:
URL: https://github.com/apache/calcite/pull/5184#discussion_r3811067534
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -3024,6 +3025,124 @@ 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)");
Review Comment:
@mihaibudiu since [CALCITE-7725](https://issues.apache.org/jira/browse/CALCITE-7725) might be more tricky than anticipated (and it seems it require other changes, e.g. [CALCITE-7728](https://issues.apache.org/jira/browse/CALCITE-7728) ), shall we move on with this PR, which seems in a good shape? (and then eventually a few tests introduced here would be adjusted when those other tickets get done).
Or do you think those ones could be done quick enough, so this PR does not wait too much?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7722] RexSimplify IS [NOT] NULL on a safe operator with Strong policy ANY and unsafe operands can be further simplified [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5184:
URL: https://github.com/apache/calcite/pull/5184#issuecomment-5373385558
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5184&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5184&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [86.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5184&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5184)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org