[GitHub] [iceberg] kbendick commented on a diff in pull request #5049: Spark: Fixes NPE when convert NOT IN filter

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

kbendick commented on code in PR #5049:
URL: https://github.com/apache/iceberg/pull/5049#discussion_r898195347


##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/SparkFilters.java:
##########
@@ -178,6 +178,7 @@ public static Expression convert(Filter filter) {
             In childInFilter = (In) childFilter;
             Expression notIn = notIn(unquote(childInFilter.attribute()),
                 Stream.of(childInFilter.values())
+                    .filter(Objects::nonNull)

Review Comment:
   I think this should potentially be handled elsewhere.
   
   As you said, this changes the original filter's meaning (removes null from the list of values), which is arguably a potentially common use case such as `select * from tbl where id not in (null, 'N/A', 'unknown')` (or some other representation of null that people might be using upstream).
   
   Does the NPE go away if you update the actual test that the `notIn` predicate winds up using (I think this is it): https://github.com/apache/iceberg/blob/98dc9fe8724375e8b932e7e8afdb925c897b4695/api/src/main/java/org/apache/iceberg/expressions/BoundSetPredicate.java#L56-L66



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