(iceberg-python) branch main updated: Fix strict NotEqualTo/NotIn pruning with partial nulls or NaNs (#3521)

[ke...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kevinjqliu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git


The following commit(s) were added to refs/heads/main by this push:
     new d0a9b91eb Fix strict NotEqualTo/NotIn pruning with partial nulls or NaNs (#3521)
d0a9b91eb is described below

commit d0a9b91eb59a5bef8a7d1603cd20ae7a4fc33895
Author: Tanmay Rauth <ta...@gmail.com>
AuthorDate: Sun Jun 21 21:10:07 2026 -0700

    Fix strict NotEqualTo/NotIn pruning with partial nulls or NaNs (#3521)
    
    ## Summary
    
    Related to #3498
    
    Fix strict metrics evaluation for `NotEqualTo` and `NotIn` so files are
    only proven to match when a column contains only nulls or only NaNs.
    Mixed null/NaN files now continue through the existing bounds checks
    instead of being treated as `ROWS_MUST_MATCH`.
    
    ## Root Cause
    
    The strict evaluator used `_can_contain_nulls` / `_can_contain_nans` for
    negative predicates. That is too broad: a file with values like `[null,
    5]` and bounds `5..5` cannot be proven to match `x != 5` or `x not in
    {5}` because the non-null row may still fail the predicate.
    
    ## Java Parity
    
    This matches Java's `StrictMetricsEvaluator`, which only short-circuits
    negative predicates when the column contains only nulls or only NaNs:
    
    -
    [`notEq`](https://github.com/apache/iceberg/blob/0b30919372df34afb632f037df88c05cdba0b134/api/src/main/java/org/apache/iceberg/expressions/StrictMetricsEvaluator.java#L341-L375)
    -
    [`notIn`](https://github.com/apache/iceberg/blob/0b30919372df34afb632f037df88c05cdba0b134/api/src/main/java/org/apache/iceberg/expressions/StrictMetricsEvaluator.java#L418-L462)
    
    ## Validation
    
    - `UV_CACHE_DIR=.cache/uv PYTHON_GIL=1 PYTHONPATH=. uv run pytest
    tests/expressions/test_evaluator.py -k "mixed_nulls_and_matching_bounds
    or mixed_nans_and_matching_bounds or all_nulls or all_nans or
    strict_integer_not_in"`
    - `UV_CACHE_DIR=.cache/uv PYTHON_GIL=1 PYTHONPATH=. uv run pytest
    tests/expressions/test_evaluator.py`
    - `UV_CACHE_DIR=.cache/uv PYTHON_GIL=1 PYTHONPATH=. uv run ruff check
    pyiceberg/expressions/visitors.py tests/expressions/test_evaluator.py`
    - `git diff --check`
    
    ---------
    
    Co-authored-by: Kevin Liu <ke...@users.noreply.github.com>
---
 pyiceberg/expressions/visitors.py   | 12 ++++++++++--
 tests/expressions/test_evaluator.py | 38 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/pyiceberg/expressions/visitors.py b/pyiceberg/expressions/visitors.py
index 817160af6..320cd3110 100644
--- a/pyiceberg/expressions/visitors.py
+++ b/pyiceberg/expressions/visitors.py
@@ -1668,7 +1668,11 @@ class _StrictMetricsEvaluator(_MetricsEvaluator):
         # 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):
+        # 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.
+        if self._contains_nulls_only(field_id) or self._contains_nans_only(field_id):
             return ROWS_MUST_MATCH
 
         field = self._get_field(field_id)
@@ -1728,7 +1732,11 @@ class _StrictMetricsEvaluator(_MetricsEvaluator):
     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):
+        # 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.
+        if self._contains_nulls_only(field_id) or self._contains_nans_only(field_id):
             return ROWS_MUST_MATCH
 
         field = self._get_field(field_id)
diff --git a/tests/expressions/test_evaluator.py b/tests/expressions/test_evaluator.py
index ea1bef0a7..47a9607f5 100644
--- a/tests/expressions/test_evaluator.py
+++ b/tests/expressions/test_evaluator.py
@@ -1523,12 +1523,48 @@ def test_strict_integer_not_in(strict_data_file_schema: Schema, strict_data_file
     assert should_read, "Should match: notIn on all nulls column"
 
     should_read = _StrictMetricsEvaluator(strict_data_file_schema, NotIn("some_nulls", {"abc", "def"})).eval(strict_data_file_1)
-    assert should_read, "Should match: notIn on some nulls column, 'bbb' > 'abc' and 'bbb' < 'def'"
+    assert not should_read, "Should not match: mixed-null notIn cannot be proven when bounds are missing"
 
     should_read = _StrictMetricsEvaluator(strict_data_file_schema, NotIn("no_nulls", {"abc", "def"})).eval(strict_data_file_1)
     assert not should_read, "Should not match: no_nulls field does not have bounds"
 
 
+def test_strict_not_eq_partial_nulls_within_bounds() -> None:
+    # Regression test for https://github.com/apache/iceberg-python/issues/3498
+    # A column that contains *some* nulls (but not only nulls) whose bounds still cover the
+    # literal must not be reported as ROWS_MUST_MATCH: the non-null value equal to the literal
+    # does not satisfy the predicate. Reporting a match here lets _DeleteFiles drop the whole
+    # data file and silently lose the row that should have survived the delete.
+    schema = Schema(NestedField(1, "x", IntegerType(), required=False))
+    data_file = DataFile.from_args(
+        file_path="file.parquet",
+        file_format=FileFormat.PARQUET,
+        partition=Record(),
+        record_count=2,
+        value_counts={1: 2},
+        null_value_counts={1: 1},  # one null, one non-null -> not "nulls only"
+        nan_value_counts={},
+        lower_bounds={1: to_bytes(IntegerType(), 5)},
+        upper_bounds={1: to_bytes(IntegerType(), 5)},  # the only non-null value is 5
+    )
+
+    assert not _StrictMetricsEvaluator(schema, NotEqualTo("x", 5)).eval(data_file), (
+        "Should not match: the non-null value 5 does not satisfy x != 5"
+    )
+    assert not _StrictMetricsEvaluator(schema, NotIn("x", {5})).eval(data_file), (
+        "Should not match: the non-null value 5 is in {5}"
+    )
+
+    # The literal sits outside the bounds, so every non-null value satisfies the predicate and
+    # the remaining nulls/NaNs also satisfy it -> the whole file matches.
+    assert _StrictMetricsEvaluator(schema, NotEqualTo("x", 6)).eval(data_file), (
+        "Should match: no value equals 6 and nulls satisfy x != 6"
+    )
+    assert _StrictMetricsEvaluator(schema, NotIn("x", {6})).eval(data_file), (
+        "Should match: no value is in {6} and nulls satisfy not-in"
+    )
+
+
 @pytest.mark.parametrize(
     "file_type, evolved_type, lower_bound, upper_bound, op, lit, expected",
     [