You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "tanmayrauth (via GitHub)" <gi...@apache.org> on 2026/06/17 07:03:10 UTC
[PR] Fix strict metrics evaluator over-pruning files for NotEqualTo/NotIn with partial nulls [iceberg-python]
tanmayrauth opened a new pull request, #3521:
URL: https://github.com/apache/iceberg-python/pull/3521
_StrictMetricsEvaluator.visit_not_equal and visit_not_in short-circuited on _can_contain_nulls / _can_contain_nans (null/NaN count > 0) and returned ROWS_MUST_MATCH without checking the value bounds. A file holding any null or NaN was therefore reported as fully matching the predicate, even when a non-null value inside the bounds did not match.
This drives _DeleteFiles (table/update/snapshot.py): ROWS_MUST_MATCH drops the whole data file without rewriting it. So delete(NotEqualTo("x", 5)) against a file with stats [null, 5] and bounds lower=upper=5 would delete the entire file, silently losing the row with value 5 that should have survived.
Every other strict ROWS_MUST_MATCH path already guards on the "only" variants (_contains_nulls_only / _contains_nans_only), matching the reference StrictMetricsEvaluator. Switch both methods to the same guard so that an all-null/all-NaN column still short-circuits to ROWS_MUST_MATCH (those rows satisfy not-equal/not-in), while a partially-null column falls through to the bounds check.
Update the existing NotIn-on-some-nulls test that encoded the buggy result and add a regression test covering the [null, value] / bounds-include-literal case for both NotEqualTo and NotIn.
Fixes #3498 (partially)
<!--
Thanks for opening a pull request!
-->
<!-- In the case this PR will resolve an issue, please replace ${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->
# Rationale for this change
## Are these changes tested?
## Are there any user-facing changes?
<!-- In the case of user-facing changes, please add the changelog label. -->
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix strict NotEqualTo/NotIn pruning with partial nulls or NaNs [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #3521:
URL: https://github.com/apache/iceberg-python/pull/3521
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix strict metrics evaluator over-pruning files for NotEqualTo/NotIn with partial nulls [iceberg-python]
Posted by "tanmayrauth (via GitHub)" <gi...@apache.org>.
tanmayrauth commented on PR #3521:
URL: https://github.com/apache/iceberg-python/pull/3521#issuecomment-4733020994
@kevinjqliu Can you please take a look when you have a moment?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix strict metrics evaluator over-pruning files for NotEqualTo/NotIn with partial nulls [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Fix strict NotEqualTo/NotIn pruning with partial nulls or NaNs [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #3521:
URL: https://github.com/apache/iceberg-python/pull/3521#discussion_r3449737998
##########
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: some non-null value may be == 'def' which is within bounds ['bbb', 'eee']"
Review Comment:
```suggestion
assert not should_read, "Should not match: mixed-null notIn cannot be proven when bounds are missing"
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org