Fokko commented on code in PR #4032:
URL: https://github.com/apache/iceberg-python/pull/4032#discussion_r4159579769
##########
pyiceberg/expressions/visitors.py:
##########
@@ -1388,23 +1388,26 @@ def visit_in(self, term: BoundTerm, literals: set[L])
-> bool:
if self._contains_nulls_only(field_id) or
self._contains_nans_only(field_id):
return ROWS_CANNOT_MATCH
- if len(literals) > IN_PREDICATE_LIMIT:
- # skip evaluating the predicate if the number of values is too big
- return ROWS_MIGHT_MATCH
-
if not isinstance(field.field_type, PrimitiveType):
raise ValueError(f"Expected PrimitiveType: {field.field_type}")
+ # only compare the smallest and largest values if the number of values
is too big
+ above_limit = len(literals) > IN_PREDICATE_LIMIT
+
lower_bound_bytes = self.lower_bounds.get(field_id)
if lower_bound_bytes is not None:
lower_bound = from_bytes(field.field_type, lower_bound_bytes)
if self._is_nan(lower_bound):
# NaN indicates unreliable bounds. See the
InclusiveMetricsEvaluator docs for more.
return ROWS_MIGHT_MATCH
- literals = {lit for lit in literals if lower_bound <= lit} #
type: ignore[operator]
- if len(literals) == 0:
- return ROWS_CANNOT_MATCH
+ if above_limit:
+ if max(literals) < lower_bound: # type: ignore[operator]
Review Comment:
I believe this should be `<=` in order to compare truncated min/max.
```suggestion
if max(literals) <= lower_bound: # type: ignore[operator]
```
--
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]
