kevinjqliu commented on code in PR #3521:
URL: https://github.com/apache/iceberg-python/pull/3521#discussion_r3449733289
##########
pyiceberg/expressions/visitors.py:
##########
@@ -1668,7 +1668,10 @@ def visit_not_equal(self, term: BoundTerm, literal:
LiteralValue) -> bool:
# Rows must match when X < Min or Max < X because it is not in the
range
field_id = term.ref().field.field_id
- if self._can_contain_nulls(field_id) or
self._can_contain_nans(field_id):
+ # When the column is entirely null or entirely NaN there is no value
equal to the
+ # literal, so every row satisfies the predicate. A merely partial
presence of nulls
+ # or NaNs does not guarantee this, so we must fall through to the
bounds check below.
Review Comment:
```suggestion
# If metrics prove the column contains only nulls or only NaNs, no
row can have
# a value equal to the literal, so every row satisfies NotEqualTo.
Partial
# null/NaN counts are not enough: a remaining non-null/non-NaN value
may still
# equal the literal, so fall through to the bounds checks.
```
##########
pyiceberg/expressions/visitors.py:
##########
@@ -1728,7 +1731,10 @@ def visit_in(self, term: BoundTerm, literals: set[L]) ->
bool:
def visit_not_in(self, term: BoundTerm, literals: set[L]) -> bool:
field_id = term.ref().field.field_id
- if self._can_contain_nulls(field_id) or
self._can_contain_nans(field_id):
+ # When the column is entirely null or entirely NaN none of the values
are in the set,
+ # so every row satisfies the predicate. A merely partial presence of
nulls or NaNs does
+ # not guarantee this, so we must fall through to the bounds check
below.
Review Comment:
```suggestion
# If metrics prove the column contains only nulls or only NaNs, no
row can have
# a value in the literal set, so every row satisfies NotIn. Partial
null/NaN
# counts are not enough: a remaining non-null/non-NaN value may
still be in the
# set, so fall through to the bounds checks.
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
