← 输入框|github.com|刷新
Skip to content

Apply every delete file when rewriting files for a partial delete - #4292

Open
ghoshp83 wants to merge 1 commit into
apache:mainfrom
ghoshp83:fix-4291-delete-keeps-all-delete-files
Open

ghoshp83 wants to merge 1 commit into
apache:mainfrom
ghoshp83:fix-4291-delete-keeps-all-delete-files

Conversation

@ghoshp83

@ghoshp83 ghoshp83 commented Nov 5, 2026

Copy link
Copy Markdown

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.

A partial delete reads each candidate file in full and writes back the
surviving rows, but planned the rewrite with the delete filter, which also
prunes delete files by column bounds. Rows removed by a pruned delete file
came back in the rewritten file. Plan the rewrite without pruning delete
files; scans are unchanged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Partial deletes can bring back rows removed by earlier delete files

1 participant