rdblue commented on a change in pull request #1747:
URL: https://github.com/apache/iceberg/pull/1747#discussion_r529013088
##########
File path:
api/src/main/java/org/apache/iceberg/expressions/InclusiveMetricsEvaluator.java
##########
@@ -150,6 +152,53 @@ public Boolean or(Boolean leftResult, Boolean rightResult)
{
return ROWS_MIGHT_MATCH;
}
+ @Override
+ public <T> Boolean isNaN(BoundReference<T> ref) {
+ Integer id = ref.fieldId();
+
+ if (nanCounts != null && nanCounts.containsKey(id) && nanCounts.get(id)
== 0) {
+ return ROWS_CANNOT_MATCH;
+ }
+
+ // when there's no nanCounts information, but we already know the column
only contains null,
+ // it's guaranteed that there's no NaN value
+ if (containsNullsOnly(id)) {
Review comment:
I agree with the reasoning. If we have NaN counts, then we should have
null counts. No need to over-complicated the null logic with a check for when
we don't have null counts but do have NaN counts. Good catch!
----------------------------------------------------------------
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]