You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2022/01/22 01:28:48 UTC
[GitHub] [calcite] NobiGo opened a new pull request #2699: [CALCITE-4988] ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simp…
NobiGo opened a new pull request #2699:
URL: https://github.com/apache/calcite/pull/2699
…lify to (A IS NOT NULL) When A is deterministic
--
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
[GitHub] [calcite] NobiGo commented on pull request #2699: [CALCITE-4988] ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simp…
Posted by GitBox <gi...@apache.org>.
NobiGo commented on pull request #2699:
URL: https://github.com/apache/calcite/pull/2699#issuecomment-1019437525
> Does this case cover the situation like "(A is not null and B is null) or A is not null"?
@ILuffZhe This can't cover this situation. The OR condition has a different way to pull up the predicate.
--
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
[GitHub] [calcite] ILuffZhe commented on pull request #2699: [CALCITE-4988] ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simp…
Posted by GitBox <gi...@apache.org>.
ILuffZhe commented on pull request #2699:
URL: https://github.com/apache/calcite/pull/2699#issuecomment-1020205102
> > Does this case cover the situation like "(A is not null and B is null) or A is not null"?
>
> @ILuffZhe This can't cover this situation. The OR condition has a different way to pull up the predicate.
Thanks for the clarification, I see you've updated the test cases. Overall LGTM!
--
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
[GitHub] [calcite] asolimando commented on a change in pull request #2699: [CALCITE-4988] ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simp…
Posted by GitBox <gi...@apache.org>.
asolimando commented on a change in pull request #2699:
URL: https://github.com/apache/calcite/pull/2699#discussion_r805757859
##########
File path: core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
##########
@@ -2157,6 +2157,82 @@ private void checkExponentialCnf(int n) {
checkSimplify(isNotNull(cast(i2, intType)), "true");
}
+ /**
+ * Unit test for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-4988">[CALCITE-4988]
+ * ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simplify to (A IS NOT NULL)
+ * When A is deterministic</a>. */
+ @Test void testSimplifyIsNotNullWithDeterministic() {
+ // "(A IS NOT NULL OR B) AND A IS NOT NULL" when A is deterministic
+ // ==>
+ // "A IS NOT NULL"
+ SqlOperator dc = getDeterministicOperator();
+ checkSimplify2(
+ and(or(isNotNull(rexBuilder.makeCall(dc)), gt(vInt(2), literal(2))),
+ isNotNull(rexBuilder.makeCall(dc))),
+ "AND(OR(IS NOT NULL(DC()), >(?0.int2, 2)), IS NOT NULL(DC()))",
+ "IS NOT NULL(DC())");
+ }
+
+ @Test void testSimplifyIsNotNullWithDeterministic2() {
+ // "(A IS NOT NULL AND B) OR A IS NULL" when A is deterministic
+ // ==>
+ // "A IS NULL OR B"
+ SqlOperator dc = getDeterministicOperator();
+ checkSimplify(
+ or(and(isNotNull(rexBuilder.makeCall(dc)), gt(vInt(2), literal(2))),
+ isNull(rexBuilder.makeCall(dc))),
+ "OR(IS NULL(DC()), >(?0.int2, 2))");
+ }
+
+ @Test void testSimplifyIsNotNullWithNoDeterministic() {
+ // "(A IS NOT NULL OR B) AND A IS NOT NULL" when A is not deterministic
+ // ==>
+ // "(A IS NOT NULL OR B) AND A IS NOT NULL"
+ SqlOperator ndc = getNoDeterministicOperator();
+ checkSimplifyUnchanged(
+ and(or(isNotNull(rexBuilder.makeCall(ndc)), gt(vInt(2), literal(2))),
+ isNotNull(rexBuilder.makeCall(ndc))));
+ }
+
+ @Test void testSimplifyIsNotNullWithNoDeterministic2() {
Review comment:
I can't spot any difference w.r.t. `testSimplifyIsNotNullWithNoDeterministic`, is this a copy-paste error or am I missing something?
--
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
[GitHub] [calcite] asolimando commented on a change in pull request #2699: [CALCITE-4988] ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simp…
Posted by GitBox <gi...@apache.org>.
asolimando commented on a change in pull request #2699:
URL: https://github.com/apache/calcite/pull/2699#discussion_r805758441
##########
File path: core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
##########
@@ -2157,6 +2157,82 @@ private void checkExponentialCnf(int n) {
checkSimplify(isNotNull(cast(i2, intType)), "true");
}
+ /**
+ * Unit test for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-4988">[CALCITE-4988]
+ * ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simplify to (A IS NOT NULL)
+ * When A is deterministic</a>. */
+ @Test void testSimplifyIsNotNullWithDeterministic() {
+ // "(A IS NOT NULL OR B) AND A IS NOT NULL" when A is deterministic
+ // ==>
+ // "A IS NOT NULL"
+ SqlOperator dc = getDeterministicOperator();
+ checkSimplify2(
+ and(or(isNotNull(rexBuilder.makeCall(dc)), gt(vInt(2), literal(2))),
+ isNotNull(rexBuilder.makeCall(dc))),
+ "AND(OR(IS NOT NULL(DC()), >(?0.int2, 2)), IS NOT NULL(DC()))",
+ "IS NOT NULL(DC())");
+ }
+
+ @Test void testSimplifyIsNotNullWithDeterministic2() {
+ // "(A IS NOT NULL AND B) OR A IS NULL" when A is deterministic
+ // ==>
+ // "A IS NULL OR B"
+ SqlOperator dc = getDeterministicOperator();
+ checkSimplify(
+ or(and(isNotNull(rexBuilder.makeCall(dc)), gt(vInt(2), literal(2))),
+ isNull(rexBuilder.makeCall(dc))),
+ "OR(IS NULL(DC()), >(?0.int2, 2))");
+ }
+
+ @Test void testSimplifyIsNotNullWithNoDeterministic() {
+ // "(A IS NOT NULL OR B) AND A IS NOT NULL" when A is not deterministic
+ // ==>
+ // "(A IS NOT NULL OR B) AND A IS NOT NULL"
+ SqlOperator ndc = getNoDeterministicOperator();
+ checkSimplifyUnchanged(
+ and(or(isNotNull(rexBuilder.makeCall(ndc)), gt(vInt(2), literal(2))),
+ isNotNull(rexBuilder.makeCall(ndc))));
+ }
+
+ @Test void testSimplifyIsNotNullWithNoDeterministic2() {
Review comment:
Never mind, just saw the OR vs AND, my bad!
--
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
[GitHub] [calcite] NobiGo merged pull request #2699: [CALCITE-4988] ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simp…
Posted by GitBox <gi...@apache.org>.
NobiGo merged pull request #2699:
URL: https://github.com/apache/calcite/pull/2699
--
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
[GitHub] [calcite] ILuffZhe commented on pull request #2699: [CALCITE-4988] ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simp…
Posted by GitBox <gi...@apache.org>.
ILuffZhe commented on pull request #2699:
URL: https://github.com/apache/calcite/pull/2699#issuecomment-1019124395
Does this case cover the situation like "(A is not null and B is null) or A is not null"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] NobiGo merged pull request #2699: [CALCITE-4988] ((A IS NOT NULL OR B) AND A IS NOT NULL) can't be simp…
Posted by GitBox <gi...@apache.org>.
NobiGo merged pull request #2699:
URL: https://github.com/apache/calcite/pull/2699
--
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