Skip to content

Commit d0a9b91

Browse files
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 <kevinjqliu@users.noreply.github.com>
1 parent c66d8b5 commit d0a9b91

2 files changed

Lines changed: 47 additions & 3 deletions

File tree

pyiceberg/expressions/visitors.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,7 +1668,11 @@ def visit_not_equal(self, term: BoundTerm, literal: LiteralValue) -> bool:
16681668
# Rows must match when X < Min or Max < X because it is not in the range
16691669
field_id = term.ref().field.field_id
16701670

1671-
if self._can_contain_nulls(field_id) or self._can_contain_nans(field_id):
1671+
# If metrics prove the column contains only nulls or only NaNs, no row can have
1672+
# a value equal to the literal, so every row satisfies NotEqualTo. Partial
1673+
# null/NaN counts are not enough: a remaining non-null/non-NaN value may still
1674+
# equal the literal, so fall through to the bounds checks.
1675+
if self._contains_nulls_only(field_id) or self._contains_nans_only(field_id):
16721676
return ROWS_MUST_MATCH
16731677

16741678
field = self._get_field(field_id)
@@ -1728,7 +1732,11 @@ def visit_in(self, term: BoundTerm, literals: set[L]) -> bool:
17281732
def visit_not_in(self, term: BoundTerm, literals: set[L]) -> bool:
17291733
field_id = term.ref().field.field_id
17301734

1731-
if self._can_contain_nulls(field_id) or self._can_contain_nans(field_id):
1735+
# If metrics prove the column contains only nulls or only NaNs, no row can have
1736+
# a value in the literal set, so every row satisfies NotIn. Partial null/NaN
1737+
# counts are not enough: a remaining non-null/non-NaN value may still be in the
1738+
# set, so fall through to the bounds checks.
1739+
if self._contains_nulls_only(field_id) or self._contains_nans_only(field_id):
17321740
return ROWS_MUST_MATCH
17331741

17341742
field = self._get_field(field_id)

tests/expressions/test_evaluator.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1523,12 +1523,48 @@ def test_strict_integer_not_in(strict_data_file_schema: Schema, strict_data_file
15231523
assert should_read, "Should match: notIn on all nulls column"
15241524

15251525
should_read = _StrictMetricsEvaluator(strict_data_file_schema, NotIn("some_nulls", {"abc", "def"})).eval(strict_data_file_1)
1526-
assert should_read, "Should match: notIn on some nulls column, 'bbb' > 'abc' and 'bbb' < 'def'"
1526+
assert not should_read, "Should not match: mixed-null notIn cannot be proven when bounds are missing"
15271527

15281528
should_read = _StrictMetricsEvaluator(strict_data_file_schema, NotIn("no_nulls", {"abc", "def"})).eval(strict_data_file_1)
15291529
assert not should_read, "Should not match: no_nulls field does not have bounds"
15301530

15311531

1532+
def test_strict_not_eq_partial_nulls_within_bounds() -> None:
1533+
# Regression test for https://github.com/apache/iceberg-python/issues/3498
1534+
# A column that contains *some* nulls (but not only nulls) whose bounds still cover the
1535+
# literal must not be reported as ROWS_MUST_MATCH: the non-null value equal to the literal
1536+
# does not satisfy the predicate. Reporting a match here lets _DeleteFiles drop the whole
1537+
# data file and silently lose the row that should have survived the delete.
1538+
schema = Schema(NestedField(1, "x", IntegerType(), required=False))
1539+
data_file = DataFile.from_args(
1540+
file_path="file.parquet",
1541+
file_format=FileFormat.PARQUET,
1542+
partition=Record(),
1543+
record_count=2,
1544+
value_counts={1: 2},
1545+
null_value_counts={1: 1}, # one null, one non-null -> not "nulls only"
1546+
nan_value_counts={},
1547+
lower_bounds={1: to_bytes(IntegerType(), 5)},
1548+
upper_bounds={1: to_bytes(IntegerType(), 5)}, # the only non-null value is 5
1549+
)
1550+
1551+
assert not _StrictMetricsEvaluator(schema, NotEqualTo("x", 5)).eval(data_file), (
1552+
"Should not match: the non-null value 5 does not satisfy x != 5"
1553+
)
1554+
assert not _StrictMetricsEvaluator(schema, NotIn("x", {5})).eval(data_file), (
1555+
"Should not match: the non-null value 5 is in {5}"
1556+
)
1557+
1558+
# The literal sits outside the bounds, so every non-null value satisfies the predicate and
1559+
# the remaining nulls/NaNs also satisfy it -> the whole file matches.
1560+
assert _StrictMetricsEvaluator(schema, NotEqualTo("x", 6)).eval(data_file), (
1561+
"Should match: no value equals 6 and nulls satisfy x != 6"
1562+
)
1563+
assert _StrictMetricsEvaluator(schema, NotIn("x", {6})).eval(data_file), (
1564+
"Should match: no value is in {6} and nulls satisfy not-in"
1565+
)
1566+
1567+
15321568
@pytest.mark.parametrize(
15331569
"file_type, evolved_type, lower_bound, upper_bound, op, lit, expected",
15341570
[

0 commit comments

Comments
 (0)