You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "thomasrebele (via GitHub)" <gi...@apache.org> on 2025/11/20 13:16:15 UTC
[PR] [CALCITE-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
thomasrebele opened a new pull request, #4646:
URL: https://github.com/apache/calcite/pull/4646
(no comment)
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2562102049
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1153,16 +1153,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(true);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NOT NULL(a),
+ // even if we cannot do anything else
}
if (predicates.pulledUpPredicates.contains(a)) {
return rexBuilder.makeLiteral(true);
}
if (hasCustomNullabilityRules(a.getKind())) {
- return null;
+ return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, a);
Review Comment:
We have built some very nice profiling tools which make use of the source position information. I will add more and more of it if possible.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2549074058
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -2776,6 +2776,11 @@ trueLiteral, literal(1),
checkSimplify(
isNotNull(div(vDecimalNotNull(), cast(literal(BigDecimal.valueOf(2.5)), intType))),
"true");
+
+ checkSimplify(isNull(cast(div(vIntNotNull(), literal(0)), tBigInt())),
Review Comment:
OK, thanks for your explain.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4646:
URL: https://github.com/apache/calcite/pull/4646#issuecomment-3565671242
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4646) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4646&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4646&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4646&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4646&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4646&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4646)
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2557883653
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1153,16 +1153,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(true);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NOT NULL(a),
+ // even if we cannot do anything else
}
if (predicates.pulledUpPredicates.contains(a)) {
return rexBuilder.makeLiteral(true);
}
if (hasCustomNullabilityRules(a.getKind())) {
- return null;
+ return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, a);
Review Comment:
what is the difference between returning null and creating a new call?
If a is unchanged, perhaps you should return null - less wasteful
Also, please use the makeCall version which preserves the source position.
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1212,16 +1211,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(false);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(false);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NULL(a),
Review Comment:
what does it mean "keep this simplification?"
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2563775537
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1152,20 +1152,21 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
if (!a.getType().isNullable() && isSafe) {
return rexBuilder.makeLiteral(true);
}
+ RexNode simplifiedResult = null;
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
Review Comment:
The lossless CAST simplification only applies to a cast directly below an `IS NOT NULL` (or `IS NULL` respectively). In that case, the cast would map `NULL` to `NULL` and non-NULL values to non-NULL values, so it does not change the result of the `IS NOT NULL`. The [implementation on the main branch](https://github.com/apache/calcite/blob/abd7ca25943513716d1ba2b592107e2e3f01cb46/core/src/main/java/org/apache/calcite/rex/RexSimplify.java#L1156-L1161) removes the cast as well. Your counterexample does not apply here, as it is not of the form `IS NOT NULL(CAST(...))`.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2561972948
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1153,16 +1153,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(true);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NOT NULL(a),
+ // even if we cannot do anything else
}
if (predicates.pulledUpPredicates.contains(a)) {
return rexBuilder.makeLiteral(true);
}
if (hasCustomNullabilityRules(a.getKind())) {
- return null;
+ return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, a);
Review Comment:
ok, I will do it in a separate PR which will do this for more operators.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2560764044
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1153,16 +1153,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(true);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NOT NULL(a),
+ // even if we cannot do anything else
}
if (predicates.pulledUpPredicates.contains(a)) {
return rexBuilder.makeLiteral(true);
}
if (hasCustomNullabilityRules(a.getKind())) {
- return null;
+ return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, a);
Review Comment:
Let's carry the source position as much as possible, there is no cost for it, and it may be used by other rewrites in calls that may throw.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2559204828
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1153,16 +1153,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(true);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NOT NULL(a),
+ // even if we cannot do anything else
}
if (predicates.pulledUpPredicates.contains(a)) {
return rexBuilder.makeLiteral(true);
}
if (hasCustomNullabilityRules(a.getKind())) {
- return null;
+ return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, a);
Review Comment:
I've added [a comment](https://github.com/apache/calcite/pull/4646/files/1fc0329b966dcd82aaeb52bd04a09ae8781000be#diff-047d75692a99d7b5c0d9b1aefc04549ec766e0a45a895b2e04a2cfa0fa6b073eR1157-R1158) why we need to return `IS NOT NULL(a)`. If we return just `null`, we would not remove the superfluous lossless cast.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2562074730
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1153,16 +1153,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(true);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NOT NULL(a),
+ // even if we cannot do anything else
}
if (predicates.pulledUpPredicates.contains(a)) {
return rexBuilder.makeLiteral(true);
}
if (hasCustomNullabilityRules(a.getKind())) {
- return null;
+ return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, a);
Review Comment:
Thank you, I've updated the PR. I'm not sure whether it's worth to spend much time on adding the source position, if it is not used afterwards.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4646:
URL: https://github.com/apache/calcite/pull/4646#issuecomment-3578218010
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4646) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4646&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4646&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4646&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4646&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4646&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4646)
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2561968021
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1153,16 +1153,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(true);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NOT NULL(a),
+ // even if we cannot do anything else
}
if (predicates.pulledUpPredicates.contains(a)) {
return rexBuilder.makeLiteral(true);
}
if (hasCustomNullabilityRules(a.getKind())) {
- return null;
+ return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, a);
Review Comment:
The source pos for the new `IS NULL` / `IS NOT NULL` call is not available yet in the body. The param `a` is the argument to that call, not the call itself. So I would need to add another parameter to that function. I feel that this is out-of-scope for the current ticket. What do you think?
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu merged PR #4646:
URL: https://github.com/apache/calcite/pull/4646
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2559197198
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1212,16 +1211,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(false);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(false);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NULL(a),
Review Comment:
The simplification that I meant is removing the superfluous lossless cast .
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2559204828
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1153,16 +1153,15 @@ private RexNode simplifyIs(RexCall call, RexUnknownAs unknownAs) {
return rexBuilder.makeLiteral(true);
}
if (RexUtil.isLosslessCast(a)) {
- if (!a.getType().isNullable()) {
- return rexBuilder.makeLiteral(true);
- }
- return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, RexUtil.removeCast(a));
+ a = RexUtil.removeCast(a);
+ // to keep this simplification, we must return IS NOT NULL(a),
+ // even if we cannot do anything else
}
if (predicates.pulledUpPredicates.contains(a)) {
return rexBuilder.makeLiteral(true);
}
if (hasCustomNullabilityRules(a.getKind())) {
- return null;
+ return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, a);
Review Comment:
I've added [a comment](https://github.com/apache/calcite/pull/4646/files/1fc0329b966dcd82aaeb52bd04a09ae8781000be#diff-047d75692a99d7b5c0d9b1aefc04549ec766e0a45a895b2e04a2cfa0fa6b073eR1157-R1158) why we need to return `IS NOT NULL(a)`. If we return just `null`, we would not remove the superfluous lossless cast.
I'll add a check whether a has changed.
Thank you for pointing out that there exist a version of makeCall that allows to specify the source position. The [existing fallback](https://github.com/thomasrebele/calcite/blob/1fc0329b966dcd82aaeb52bd04a09ae8781000be/core/src/main/java/org/apache/calcite/rex/RexSimplify.java#L1138-L1140) has no source position either. Reading [CALCITE-6071](https://issues.apache.org/jira/browse/CALCITE-6071), it seems that only calls that may throw should be annotated with the source position. As `IS NULL` / `IS NOT NULL` don't throw themselves, they would not need the source position.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2548417709
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -2776,6 +2776,11 @@ trueLiteral, literal(1),
checkSimplify(
isNotNull(div(vDecimalNotNull(), cast(literal(BigDecimal.valueOf(2.5)), intType))),
"true");
+
+ checkSimplify(isNull(cast(div(vIntNotNull(), literal(0)), tBigInt())),
Review Comment:
```vIntNotNull``` is valued 0, could we also add non-zero test for it, such as ```vIntNotNull(1)```, just a suggestion.
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #4646:
URL: https://github.com/apache/calcite/pull/4646#discussion_r2549028291
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -2776,6 +2776,11 @@ trueLiteral, literal(1),
checkSimplify(
isNotNull(div(vDecimalNotNull(), cast(literal(BigDecimal.valueOf(2.5)), intType))),
"true");
+
+ checkSimplify(isNull(cast(div(vIntNotNull(), literal(0)), tBigInt())),
Review Comment:
As far as I know, `vIntNotNull()` has no specified value. It is just a dynamic parameter (`?0.notNullInt0`). Adding a paramater would change the index, so `vIntNotNull(1)` would become a `?0.notNullInt1`. The only place that I've seen that the dynamic parameters get assigned a value is in `org.apache.calcite.rex.RexSimplify#verify`, and that includes several constants: -1, 0, 1, and 1000000 (see `org.apache.calcite.rex.RexAnalyzer#getComparables`).
--
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-7296] RexSimplify should not simplify IS NULL(CAST(10/0 AS BIGINT)) [calcite]
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on PR #4646:
URL: https://github.com/apache/calcite/pull/4646#issuecomment-3587219351
Thank you for the reviews, @mihaibudiu and @xuzifu666!
--
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