azwanzuharimi opened a new pull request, #4032:
URL: https://github.com/apache/iceberg-python/pull/4032
Closes #4003
# Rationale for this change
`_InclusiveMetricsEvaluationVisitor.visit_in` and
`_ManifestEvalVisitor.visit_in` return `ROWS_MIGHT_MATCH` as soon as an `In`
predicate has more than `IN_PREDICATE_LIMIT` (200) values. They return before
they read any bounds. So `plan_files` keeps every data file above that limit. A
delete or an upsert with a large key set then scans the whole table. #4003
measured this: 200 keys plan 1 file out of 20, 201 keys plan all 20.
The limit copies Java `InclusiveMetricsEvaluator`, where apache/iceberg#1672
added it in 2020. The concern was the cost of filtering every literal per file.
This change keeps the exact check at 200 values or fewer. Above the limit it
compares only `min(literals)` and `max(literals)` against the file bounds:
- `max(literals) < lower_bound` gives `ROWS_CANNOT_MATCH`.
- `min(literals) > upper_bound` gives `ROWS_CANNOT_MATCH`.
- Every other case gives `ROWS_MIGHT_MATCH`.
The result above the limit is always a superset of the exact check. A file
that the exact check keeps is never pruned. The exact check can also prune a
file when all values sit outside the bounds on both sides. The min and max
check keeps that file. A test records this.
The two visitors differ:
- `_ManifestEvalVisitor` tests the full set with `all(...)` for each bound.
That is the same as a min and max test. So above the limit it only replaces the
set with `{min, max}` and runs the existing checks.
- `_InclusiveMetricsEvaluationVisitor` filters the set by the lower bound
before it tests the upper bound. The `{min, max}` trick would prune too much
there, so it uses an explicit branch.
Cost: `min` and `max` run once per file. Measured locally on macOS, one call
of `min` plus `max` on a set of 100,000 values takes about 1.2 ms for ints and
2.8 ms for strings. Below the limit nothing changes.
`table/update/validate.py` uses the same evaluator for conflict detection.
It drops a file only on `ROWS_CANNOT_MATCH`, so this change can only remove
false conflicts.
This diverges from Java, which still returns `ROWS_MIGHT_MATCH` above the
limit. Java can mirror it with `Collections.min` and `Collections.max` on the
literal set.
## Are these changes tested?
Yes.
- `tests/expressions/test_evaluator.py`: `test_integer_in_above_limit`,
`test_integer_in_at_limit` and
`test_inclusive_metrics_evaluator_in_above_limit`. They cover values below,
above, equal to and across the bounds, the 200 value boundary, an all nulls
column, and NaN bounds.
- `tests/expressions/test_visitors.py`: `test_integer_in_above_limit` for
the manifest evaluator.
The above limit tests fail on `main`. `make lint` passes.
`tests/expressions` and `tests/table/test_evaluator_planning.py` pass with 172
tests.
## Are there any user-facing changes?
No API change. An `In` predicate with more than 200 values now prunes files
by bounds. Scans, deletes and upserts with large key sets read fewer files.
https://claude.ai/code/session_01XwKs9csmvNooLVfK61qYwA
--
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]
