ptimizeroracle opened a new pull request, #3921:
URL: https://github.com/apache/iceberg-python/pull/3921
# Rationale for this change
`_StrictMetricsEvaluationVisitor.visit_not_in` returns a false
`ROWS_MUST_MATCH` when the
upper bound of a float column is NaN.
`NaN >= val` is `False` for every value, so the `upper >= val` filter drops
every literal
from the set and the empty set is read as proof that no row can match. But a
NaN upper
bound only means NaN sorts greatest in the writer's min/max: the file can
still contain
non-NaN values that are in the literal set.
Reproducer (offline, no catalog):
```python
schema = Schema(NestedField(1, "x", DoubleType(), required=False))
data_file = DataFile.from_args(
file_path="file.parquet",
file_format=FileFormat.PARQUET,
partition={},
record_count=2,
file_size_in_bytes=1,
value_counts={1: 2},
null_value_counts={1: 0},
nan_value_counts={1: 1},
lower_bounds={1: to_bytes(DoubleType(), 1.0)},
upper_bounds={1: to_bytes(DoubleType(), float("nan"))},
)
_StrictMetricsEvaluator(schema, NotIn("x", {1.0, 2.0})).eval(data_file)
# main: True (ROWS_MUST_MATCH)
# fixed: False (ROWS_MIGHT_NOT_MATCH)
```
The file describes a column `{1.0, NaN}`. Evaluating the predicate on the
actual rows
with `expression_evaluator` gives `[False, True]`: the row `1.0` does not
match
`NotIn("x", {1.0, 2.0})`, so `ROWS_MUST_MATCH` is impossible.
This is not just a wrong pruning verdict. `_DeleteFiles._compute_deletes`
(pyiceberg/table/update/snapshot.py:612) drops a whole data file when the
strict
evaluator returns `ROWS_MUST_MATCH`, so a `Table.delete(...)` with a `NotIn`
filter can
delete rows that should be kept.
Java parity: `StrictEvalVisitor.notIn`
(api/src/main/java/org/apache/iceberg/expressions/StrictEvalVisitor.java:358-383)
guards the lower bound explicitly with `NaNUtil.isNaN(lower)` and its
comparator orders
NaN greatest, so a NaN upper bound never excludes literals there. The Python
port
already mirrors the lower-bound guard (visitors.py:1774); this adds the
missing
upper-bound guard with the same comment, matching the Java docs note that
NaN bounds
are unreliable when the column can contain non-NaN data.
Same genre as #3891 (NaN/null-bound handling in the metrics evaluators).
## Are these changes tested?
Yes. `test_strict_not_in_with_nan_upper_bound` in
tests/expressions/test_evaluator.py,
parametrized over FloatType/DoubleType, modeled on the neighboring
`test_strict_not_equal_and_not_in_with_mixed_nans_and_matching_bounds`. It
fails on
main and passes with the fix. Full unit suite: 4019 passed, 3 skipped.
## Are there any user-facing changes?
Yes, bug fix: `Table.delete` with a `NotIn` row filter no longer drops data
files whose
float column has a NaN upper bound and partially-matching rows. No API
changes.
--
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]
