[GitHub] [calcite] vlsi commented on a change in pull request #2357: [CALCITE-4446] Implement three-valued logic for SEARCH operator

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

vlsi commented on a change in pull request #2357:
URL: https://github.com/apache/calcite/pull/2357#discussion_r584913833



##########
File path: core/src/main/java/org/apache/calcite/rex/RexSimplify.java
##########
@@ -2850,12 +2838,46 @@ static RexNode fix(RexBuilder rexBuilder, RexNode term) {
 
     @SuppressWarnings({"rawtypes", "unchecked", "UnstableApiUsage"})
     <C extends Comparable<C>> Sarg<C> build(boolean negate) {
+      final RexUnknownAs unknownAs = deriveUnknownAs();
+      final RangeSet<C> r = (RangeSet) this.rangeSet;
       if (negate) {
-        return Sarg.of(notNullTermCount == 0,
-            (RangeSet) rangeSet.complement());
+        return Sarg.of(unknownAs.negate(), r.complement());
       } else {
-        return Sarg.of(nullTermCount > 0, (RangeSet) rangeSet);
+        return Sarg.of(unknownAs, r);
+      }
+    }
+
+    /** Derives the value that will be assigned to {@link Sarg#nullAs}.
+     *
+     * <p>Behavior is as follows:
+     *
+     * <ul>
+     * <li>If there is at least one term that returns TRUE when the argument
+     * is NULL, then the overall value will be TRUE; failing that,
+     * <li>if there is at least one term that returns UNKNOWN when the argument
+     * is NULL, then the overall value will be UNKNOWN; failing that,
+     * <li>the value will be FALSE.
+     * </ul>
+     *
+     * <p>Consider the behavior of an OR expression in terms of three-valued
+     * logic:
+     * {@code TRUE OR UNKNOWN OR FALSE} returns {@code TRUE};
+     * {@code UNKNOWN OR FALSE OR UNKNOWN} returns {@code UNKNOWN};
+     * {@code FALSE OR FALSE} returns {@code FALSE}.
+     * It will be evident that a Sarg that is the union of several terms (each
+     * of which can be seen as a Sarg with its own null behavior) will behave
+     * analogously. */

Review comment:
       As far as I understand, this means `the union of Sarg1  Sarg2 behaves exactly the same as Sarg` OR Sarg2`.
   
   If that is the case, then please describe it like that. We don't need to count true, false or whatever.
   
   Since we never remove an entry from SargCollector, the overall `unknownAs` can start from `null`, and then gradually climp up to `FALSE`, or `UNKNOWN`, or even `TRUE`. The counters are not needed, and they make code hard to understand.




----------------------------------------------------------------
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