You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "macroguo-ghy (via GitHub)" <gi...@apache.org> on 2023/09/10 09:30:43 UTC
[GitHub] [calcite] macroguo-ghy opened a new pull request, #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
macroguo-ghy opened a new pull request, #3416:
URL: https://github.com/apache/calcite/pull/3416
Simplify 'p OR NOT p' to 'TRUE' (if p is not nullable), or to 'null AND p IS NOT NULL' (if p is nullable).
GeodeFilter support converting 'p IS NOT NULL' to 'p <> 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] macroguo-ghy commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1334093135
##########
geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java:
##########
@@ -161,7 +161,7 @@ private CalciteAssert.AssertThat calciteAssert() {
GeodeAssertions.query("SELECT stringValue AS stringValue "
+ "FROM /allDataTypesRegion WHERE "
+ "stringValue IN SET('abc', 'def') OR floatValue = 1.5678 "
- + "OR booleanValue <> null"));
+ + "OR booleanValue = true OR booleanValue = false"));
Review Comment:
In Geodo adapter, all types are represented as JavaType, so all refs will be wrapped with `CAST`.
However, `CAST` is not a safe expression as we cannot guarantee the success or failure of the cast in some cases.
For example:
```sql
cast('2023-03-23' as date) -- success
cast('aaaa' as date) -- fail
```
Both of these statements attempt to cast a varchar as a date, but the first one is legal while the second one will fail.
But I am not familiar with geodo adapters, so I am not very clear why all types are using Java types, I only discovered this during debugging.
--
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] macroguo-ghy commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1323207991
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
+ }
+ break;
+ case NOT:
+ RexNode x = ((RexCall) term).getOperands().get(0);
+ if (terms.contains(x)) {
+ if (!x.getType().isNullable()) {
+ return trueLiteral;
+ }
+
+ final RexNode isNotNull =
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, x);
+ terms.set(terms.indexOf(x), simplifyIs((RexCall) isNotNull, unknownAs));
+ terms.set(i, rexBuilder.makeNullLiteral(x.getType()));
+ i--;
Review Comment:
To simplify `null` literal in next for loop.
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
+ }
+ break;
+ case NOT:
+ RexNode x = ((RexCall) term).getOperands().get(0);
+ if (terms.contains(x)) {
Review Comment:
@kgyrtkirk Thanks for your review!
All operands in `OR` operator must be `boolean` type (base on the operand checker), `x` could be `1 / 0 <> 0` but could not be `1 / 0`. So I think wo don't need to care about if `x` safe or not. WDYT?
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
Review Comment:
Same viewpoint as my other 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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1722244273
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3416)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list)
--
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] kgyrtkirk commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1321568345
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
Review Comment:
I'm not sure about this; I think safety should still be considered:
```
1/0 or x or (x is not true)
```
I think even if its okay for `x` ; we can't replace it with `true` unless all other parts of the or passes `isSafeExpression` or something similar
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
+ }
+ break;
+ case NOT:
+ RexNode x = ((RexCall) term).getOperands().get(0);
+ if (terms.contains(x)) {
Review Comment:
I think to proceed in both cases `x` must be `isSafeExpression` to remove them
##########
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##########
@@ -2318,7 +2318,7 @@ trueLiteral, literal(2),
isTrue(vBool()), literal(1),
isNotTrue(vBool()), literal(1),
literal(2)),
- "CASE(OR(?0.bool0, IS NOT TRUE(?0.bool0)), 1, 2)");
+ "1");
Review Comment:
:cowboy_hat_face:
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
+ }
+ break;
+ case NOT:
+ RexNode x = ((RexCall) term).getOperands().get(0);
+ if (terms.contains(x)) {
+ if (!x.getType().isNullable()) {
+ return trueLiteral;
+ }
+
+ final RexNode isNotNull =
+ rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, x);
+ terms.set(terms.indexOf(x), simplifyIs((RexCall) isNotNull, unknownAs));
+ terms.set(i, rexBuilder.makeNullLiteral(x.getType()));
+ i--;
Review Comment:
I don't see the reason for this `i--` why its needed?
--
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] macroguo-ghy commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1722256355
Hi, @kgyrtkirk. Sorry for late of response, I was a little busy this week.
I did some changes, would you like to review this PR 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
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1712769176
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3416)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1712839355
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3416)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list)
--
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] kgyrtkirk commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1727113203
I don't want to hold you back - this is really usefull; I'm a bit sad to see that the simplification opportunity was missed in the geode adapter ... but the goal here was to handle `p || !p` - which looks good!
--
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] sonarcloud[bot] commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1722255036
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3416)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list)
--
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-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE' [calcite]
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk merged PR #3416:
URL: https://github.com/apache/calcite/pull/3416
--
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] kgyrtkirk commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1331104933
##########
geode/src/test/java/org/apache/calcite/adapter/geode/rel/GeodeAllDataTypesTest.java:
##########
@@ -161,7 +161,7 @@ private CalciteAssert.AssertThat calciteAssert() {
GeodeAssertions.query("SELECT stringValue AS stringValue "
+ "FROM /allDataTypesRegion WHERE "
+ "stringValue IN SET('abc', 'def') OR floatValue = 1.5678 "
- + "OR booleanValue <> null"));
+ + "OR booleanValue = true OR booleanValue = false"));
Review Comment:
I wonder why this is not getting rewritten -
why `booleanValue` isn't considered safe? (it seems like a column to me - isn't it)
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
Review Comment:
I understand your point - and yes that's kinda outside the scope of this PR - interesting to know that other systems are discarding them.
> And third, in most of programing language(like Java, C++, Python),
I disagree - I think they may not do that; see the following test:
```
int q=0;
@Test
public void asd ( ) {
RuntimeException e = assertThrows(RuntimeException.class, () ->
{if(crapMethodWithSideEffects() || crapThrowingException() || true) {
System.out.println("yippie");
}
});
assertEquals(1,q);
}
public boolean crapThrowingException() {
throw new RuntimeException();
}
public boolean crapMethodWithSideEffects() {
q=1;
return false;
}
```
you can try C/C++ I think that will also follow left-to-right in this case; I think the removal of such method may only happen if it could be proved that no sideeffects can occur
For the above reasons I have a bad feeling in removing unknown things...
--
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] macroguo-ghy commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1327973848
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
+ }
+ break;
+ case NOT:
+ RexNode x = ((RexCall) term).getOperands().get(0);
+ if (terms.contains(x)) {
Review Comment:
Thanks for your explaination, I got your points.
For the first case `x or not(x)`, I add a new limitation where `x` must be safeExpression.
But for the second case `a or x or not(x) or b`, I think we can simplify `x or not(x)` no matter whether `a` and `b` safe or not.
Thinking about a simple case: `true or unSafeUdf()`, calcite will simplify this expression to `true`. And I have tested `select * from t where 1/0 or true` expression in mysql and sparksql, both of them will simpilfy this expression to `true`. And third, in most of programing language (like Java, C++, Python), they always simplify `true or someConditions` to `true` in if clause (A little different, it is Short-circuit OR actully, but we check all items).
So I think we don't need to care about outside items whether safe or not.
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
Review Comment:
Thanks for your explaination, I got your points.
For the first case `x or not(x)`, I add a new limitation where `x` must be safeExpression.
But for the second case `a or x or not(x) or b`, I think we can simplify x or not(x) no matter whether `a` and `b` safe or not.
Thinking about a simple case: `true or unSafeUdf()`, calcite will simplify this expression to `true`. And I have tested `select * from t where 1/0 or true` expression in mysql and sparksql, both of them will simpilfy this expression to `true`. And third, in most of programing language(like Java, C++, Python), they always simplify `true or someConditions` to `true` in if clause (A little different, it is Short-circuit OR actully, but we check all items).
So I think we don't need to care about outside items whether safe or not.
--
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] sonarcloud[bot] commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1731159425
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3416)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list)
--
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] macroguo-ghy commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1334065232
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
Review Comment:
Yes, you are correct. In programming languages, the evaluation of `||` follows a left-to-right order. However, in SQL, we need to check all items in the OR condition. So I say it is "a little different".
And I mentioned a terminology [`Short-circuit OR`](https://chortle.ccsu.edu/java5/Notes/chap40/ch40_8.html), which means if the first condition evaluates to true, the subsequent conditions are not evaluated because the overall result of the OR operation is already determined to be true.
So I think we are consistent in how programming languages simplify `ORs` :)
--
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] sonarcloud[bot] commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1713612565
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3416)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [6 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list)
--
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] sonarcloud[bot] commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1713655572
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3416)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list)
--
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] thomasrebele commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "thomasrebele (via GitHub)" <gi...@apache.org>.
thomasrebele commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1321133695
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1993,6 +1993,7 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
// or 'X IS NOT NULL' if a simplification takes place (because another 'X<>B' is found)
final Map<RexNode, RexNode> notEqualsComparisonMap = new HashMap<>();
final RexLiteral trueLiteral = rexBuilder.makeLiteral(true);
+ boolean removeNull = false;
Review Comment:
I would rather use a flag `hasNullTerm`, and also set it in the `case LITERAL` if the case does not remove it.
--
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] macroguo-ghy commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1735455380
Hi @kgyrtkirk, could you please review this PR again or merge it?
--
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] sonarcloud[bot] commented on pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3416:
URL: https://github.com/apache/calcite/pull/3416#issuecomment-1722255920
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3416)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3416&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3416&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3416&metric=new_duplicated_lines_density&view=list)
--
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] macroguo-ghy commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "macroguo-ghy (via GitHub)" <gi...@apache.org>.
macroguo-ghy commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1321367481
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -1993,6 +1993,7 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
// or 'X IS NOT NULL' if a simplification takes place (because another 'X<>B' is found)
final Map<RexNode, RexNode> notEqualsComparisonMap = new HashMap<>();
final RexLiteral trueLiteral = rexBuilder.makeLiteral(true);
+ boolean removeNull = false;
Review Comment:
I remove this logic because I found `simplifyOrs` method will call `flattenOr` eventually which use a `set` to eliminate duplicates. So we don't need to handle multiple `NULL` literal 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
[GitHub] [calcite] julianhyde commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1327764526
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
Review Comment:
My position on safety is that only CASE has a defined evaluation order. Though we've never really implemented it.
--
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] kgyrtkirk commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1323310774
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
+ }
+ break;
+ case NOT:
+ RexNode x = ((RexCall) term).getOperands().get(0);
+ if (terms.contains(x)) {
Review Comment:
you've replied to that comment here..that's not great - because I think these are 2 separate issues - but they are somewhat similar.
`1/0` is not a boolean - my mistake; I just wanted to illustrate the problem quickly
Don't expect that `1 / 0` will appear as is ... it will be more likely `x / y > 17` which is a boolean; can't be decided at compile time that it will never or always throw exceptions - but still holds the possiblity of an exception because if `y` is `0` it will emit an exception.
I think the current patch will remove and simplify unknown udfs away - we don't know for sure that omitting those calls remove sideeffects or not.
I think
```
someUdf() or someUdf() is not true ==> true
```
is wrong because it could be a possibly unsafe method - you may not make such transformation; or at least call that method - but I would rather stay on the safe side and not do that either...
and also
```
someUdf() or x or !x ==> true (in case x is not nullable)
```
almost the same as above - however in this case the unsafe expr is outside
In the above cases: what if `someUdf` throws an exception on every invocation? should the query be executed - because we were able to deduce that its return value is irrelevant?
Could you please add some illustrative testcases for the above 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
[GitHub] [calcite] kgyrtkirk commented on a diff in pull request #3416: [CALCITE-4189] Simplify 'p OR (p IS NOT TRUE)' to 'TRUE'
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk commented on code in PR #3416:
URL: https://github.com/apache/calcite/pull/3416#discussion_r1331093076
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2060,6 +2060,25 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
}
}
break;
+ case IS_NOT_TRUE:
+ if (terms.contains(((RexCall) term).getOperands().get(0))) {
+ return trueLiteral;
Review Comment:
I understand your point - and yes that's kinda outside the scope of this PR - interesting to know that other systems are discarding them.
> And third, in most of programing language(like Java, C++, Python),
I disagree - I think they may not do that; see the following test:
```
int q=0;
@Test
public void asd ( ) {
RuntimeException e = assertThrows(RuntimeException.class, () ->
{if(crapMethodWithSideEffects() || crapThrowingException() || true) {
System.out.println("yippie");
}
});
assertEquals(1,q);
}
public boolean crapThrowingException() {
throw new RuntimeException();
}
public boolean crapMethodWithSideEffects() {
q=1;
return false;
}
```
you can try C/C++ I think that will also follow left-to-right in this case; I think the removal of such method may only happen if it could be proved that no sideeffects can occur
For the above reasons I have a bad feeling in removing unknown things...
--
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