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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
