[GitHub] [iceberg] zhongyujiang commented on a diff in pull request #6431: Parquet: Fix ParquetDictionaryRowGroupFilter evaluating NaN.

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

zhongyujiang commented on code in PR #6431:
URL: https://github.com/apache/iceberg/pull/6431#discussion_r1051749856


##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetDictionaryRowGroupFilter.java:
##########
@@ -165,20 +165,41 @@ public <T> Boolean isNaN(BoundReference<T> ref) {
       }
 
       Set<T> dictionary = dict(id, comparatorForNaNPredicate(ref));
-      return dictionary.stream().anyMatch(NaNUtil::isNaN) ? ROWS_MIGHT_MATCH : ROWS_CANNOT_MATCH;
+      return dictionary.contains(naNValueForType(ref.type()))
+          ? ROWS_MIGHT_MATCH
+          : ROWS_CANNOT_MATCH;
     }
 
     @Override
     public <T> Boolean notNaN(BoundReference<T> ref) {
       int id = ref.fieldId();
 
+      if (mayContainNulls.get(id)) {
+        return ROWS_MIGHT_MATCH;
+      }
+
       Boolean hasNonDictPage = isFallback.get(id);
       if (hasNonDictPage == null || hasNonDictPage) {
         return ROWS_MIGHT_MATCH;
       }
 
       Set<T> dictionary = dict(id, comparatorForNaNPredicate(ref));
-      return dictionary.stream().allMatch(NaNUtil::isNaN) ? ROWS_CANNOT_MATCH : ROWS_MIGHT_MATCH;
+
+      if (dictionary.size() > 1 || !dictionary.contains(naNValueForType(ref.type()))) {

Review Comment:
   I don't know how Parquet handles NaN values equality in dictionary, the reason I did this change is because  `dictionary` set is [using](https://github.com/apache/iceberg/blob/be0c8ddfd2a6d3c0f2e38b0b980d8fe73980ca3f/parquet/src/main/java/org/apache/iceberg/parquet/ParquetDictionaryRowGroupFilter.java#L184) `NaturalOrderComparator` which treats NaN values of the same type as equal, so I think there can only be at most one NaN value in this `dictionary`.



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