ghoshp83 opened a new pull request, #4292:
URL: https://github.com/apache/iceberg-python/pull/4292
Closes #4291.
# Rationale for this change
A partial `Table.delete` rewrites each data file that might hold matching
rows. It reads the whole file with `AlwaysTrue()` and writes back the rows that
don't match. The files to rewrite were planned with the delete filter, though,
and that plan also leaves out delete files whose column bounds can't match the
filter. For a filtered read that's correct, because the residual drops those
rows anyway. For a full-file rewrite it isn't: the left-out delete files are
never applied, so the rows they deleted come back in the new data file.
`overwrite(..., overwrite_filter=...)`, `dynamic_partition_overwrite` and
`upsert` go through `Transaction.delete`, so they're affected too.
The fix keeps the delete filter for choosing data files, but stops pruning
delete files by their column bounds when planning a rewrite:
- `ManifestGroupPlanner` takes `prune_delete_files: bool = True`. When it's
`False`, the metrics evaluator is applied to data entries only. Partition
pruning is unchanged, since a delete file only applies to data files in its own
partition.
- `Transaction.delete` plans its rewrite with `prune_delete_files=False`.
Its scan never has a catalog, so it always planned locally, and server-side
planning isn't involved.
Scans keep the default, so reads and their planning cost are unchanged.
# Are these changes tested?
Yes. `test_partial_delete_keeps_earlier_deletes_outside_the_filter` in
`tests/catalog/test_catalog_behaviors.py` runs against each catalog fixture and
follows case 1 from the issue:
- it commits a position delete that stores the deleted row `(2, 'b')`, which
gives it bounds `k = 2`;
- it checks that a scan for `k = 1` leaves that delete file out;
- it runs `delete("k = 1")` and checks only `k = 3` is left.
Without the fix the final assertion fails with `[2, 3] == [3]`: the deleted
row is back. PyIceberg can't write delete manifests yet, so the test's helper
subclasses `ManifestWriterV2`, as the reproduction in the issue does. Once
#4002 lands, the helper can pass `content=ManifestContent.DELETES` instead.
The equality-delete case from the issue isn't tested here, because PyIceberg
can't read equality deletes. With this change, a partial delete over a file
that has an equality delete now reaches the existing "does not yet support
equality deletes" error instead of silently writing the deleted rows back.
# Are there any user-facing changes?
Partial deletes, filtered overwrites and upserts no longer resurrect rows
removed by earlier delete files. A partial delete on a table with equality
delete files that only the column bounds used to exclude now raises the
existing unsupported-equality-deletes error, rather than writing incorrect data.
Not in this PR: the issue also points out that the retry conflict check
(`_filter_manifest_entries` in `table/update/validate.py`) prunes delete
entries the same way. That's a separate code path, so I'll follow up on it
separately.
--
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]
