[GitHub] [iceberg] rdblue commented on a change in pull request #3578: Core: Fix null handling in NOT IN in StrictMetricsEvaluator

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

rdblue commented on a change in pull request #3578:
URL: https://github.com/apache/iceberg/pull/3578#discussion_r753437126



##########
File path: api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java
##########
@@ -211,7 +211,12 @@ private Expression bindInOperation(BoundTerm<T> boundTerm) {
         case IN:
           return new BoundLiteralPredicate<>(Operation.EQ, boundTerm, Iterables.get(convertedLiterals, 0));
         case NOT_IN:
-          return new BoundLiteralPredicate<>(Operation.NOT_EQ, boundTerm, Iterables.get(convertedLiterals, 0));
+          if (boundTerm.ref().field().isRequired()) {
+            // 'col NOT IN (1)' is equivalent to 'col != 1' iff the column has no nulls

Review comment:
       `null NOT IN (1)` in SQL evaluates to `null`. Similarly, `null != 1` evaluates to `null`. Since both are `null` and both end up being `false`, aren't these expressions equivalent even if there are nulls?
   
   It seems like this is trying to work around the fact that `null != 1` evaluates to `true` for Iceberg expressions because Iceberg expressions don't use 3-value boolean logic.




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org