[GitHub] [calcite] kgyrtkirk commented on a change in pull request #1322: [CALCITE-3198] ReduceExpressionsRule.FILTER_INSTANCE does not reduce 'NOT(x=a AND x=b)'

[GitBox <gi...@apache.org>]

kgyrtkirk commented on a change in pull request #1322: [CALCITE-3198] ReduceExpressionsRule.FILTER_INSTANCE does not reduce 'NOT(x=a AND x=b)'
URL: https://github.com/apache/calcite/pull/1322#discussion_r305635221
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
 ##########
 @@ -1624,6 +1626,24 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
             continue;
           }
         }
+        break;
+      case NOT_EQUALS:
+        Comparison notEqualsComparison =
+            Comparison.of(term, node -> RexUtil.isReferenceOrAccess(node, false));
+        if (notEqualsComparison != null) {
+          Comparison prevNotEqualsComparison = notEqualsComparisonMap.get(notEqualsComparison.ref);
+          if (prevNotEqualsComparison == null) {
+            notEqualsComparisonMap.put(notEqualsComparison.ref, notEqualsComparison);
+          } else {
+            Comparable comparable1 = notEqualsComparison.literal.getValue();
+            Comparable comparable2 = prevNotEqualsComparison.literal.getValue();
+            //noinspection unchecked
+            if (comparable1.compareTo(comparable2) != 0) {
+              return rexBuilder.makeLiteral(true);
 
 Review comment:
   what if `x != 0 || x!= 1` in case x is a nullable variable?
   when `x is null`; I think the expression should evaluate to `null` instead of `true`
   
   I'm not sure if this is right now in a specific unknownas mode (doesn't seems so); or the operands are non nullables - but the best is to assume that operand of the comparision might be nullable without even considering the actual unknownAs mode.
   
   the important thing is that I think at this point you may only conclude that `x is not null or null`
   Which is `true` in some cases:
   * like when `x is of a type which is not nullable`
   * or in unknownas.TRUE mode
   
   you may narrow down this to match only the known good ones; or use the above generalized fact and let other parts of reximplify to do the rest

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services