yyanyy commented on a change in pull request #2495:
URL: https://github.com/apache/iceberg/pull/2495#discussion_r616182133
##########
File path: core/src/main/java/org/apache/iceberg/util/ManifestFileUtil.java
##########
@@ -59,16 +59,16 @@ boolean canContain(Object value) {
return containsNull;
}
- if (NaNUtil.isNaN(value)) {
- return containsNaN;
- }
-
// if lower bound is null, then there are no non-null values
if (lowerBound == null) {
// the value is non-null, so it cannot match
return false;
Review comment:
I think if the input is NaN and the file is written with the new code
(`NaN` flips `containsNaN` to true without touching upper/lower bound) then I
think we will return false here which wouldn't be correct?
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
