Skip to content

Commit 2d2df69

Browse files
committed
feat: Add metadata-only REPLACE API via _RewriteFiles snapshot producer
Adds a _RewriteFiles snapshot producer that implements Operation.REPLACE for data file compaction (bin-packing, sort, format migration). This mirrors Java Iceberg's BaseRewriteFiles and is accessed via: with table.transaction() as tx: with tx.update_snapshot().replace() as rewrite: rewrite.delete_data_file(old_file) rewrite.append_data_file(new_file) Key behaviors: - Validates all files-to-delete exist in the table - Enforces added_records <= deleted_records invariant - No-op on empty input (no snapshot produced) - Reuses unaffected manifests, rewrites only dirty ones - Passes through delete-file manifests unchanged - Scoped _validate_concurrency: only checks no new delete files conflict with replaced data (concurrent appends are NOT conflicts) - Retry-safe via _refresh_for_retry clearing @cached_property Scoped to data-file rewriting only. Delete-file rewriting, dataSequenceNumber override, and validateFromSnapshot are documented as future additive extensions. Fixes #3130
1 parent 9299bdb commit 2d2df69

4 files changed

Lines changed: 1130 additions & 82 deletions

File tree

pyiceberg/table/snapshots.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ def _partition_summary(self, update_metrics: UpdateMetrics) -> str:
351351

352352

353353
def update_snapshot_summaries(summary: Summary, previous_summary: Mapping[str, str] | None = None) -> Summary:
354-
if summary.operation not in {Operation.APPEND, Operation.OVERWRITE, Operation.DELETE}:
354+
if summary.operation not in {Operation.APPEND, Operation.OVERWRITE, Operation.DELETE, Operation.REPLACE}:
355355
raise ValueError(f"Operation not implemented: {summary.operation}")
356356

357357
if not previous_summary:

pyiceberg/table/update/snapshot.py

Lines changed: 185 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,75 @@ def _calculate_added_rows(self, manifests: list[ManifestFile]) -> int:
216216
added_rows += manifest.added_rows_count
217217
return added_rows
218218

219-
@abstractmethod
220-
def _deleted_entries(self) -> list[ManifestEntry]: ...
219+
def _get_existing_manifests(self, should_use_manifest_pruning: bool) -> list[ManifestFile]:
220+
"""Filter existing manifests and rewrite those containing deleted data files."""
221+
existing_files: list[ManifestFile] = []
222+
manifest_evaluators: dict[int, Callable[[ManifestFile], bool]] = KeyDefaultDict(self._build_manifest_evaluator)
223+
224+
if snapshot := self._transaction.table_metadata.snapshot_by_name(name=self._target_branch):
225+
for manifest_file in snapshot.manifests(io=self._io):
226+
if should_use_manifest_pruning and not manifest_evaluators[manifest_file.partition_spec_id](manifest_file):
227+
existing_files.append(manifest_file)
228+
continue
229+
230+
entries_to_write: list[ManifestEntry] = []
231+
found_deleted_entries = False
232+
233+
for entry in manifest_file.fetch_manifest_entry(io=self._io, discard_deleted=True):
234+
if entry.data_file in self._deleted_data_files:
235+
found_deleted_entries = True
236+
else:
237+
entries_to_write.append(entry)
238+
239+
if not found_deleted_entries:
240+
existing_files.append(manifest_file)
241+
continue
242+
243+
if len(entries_to_write) > 0:
244+
with self.new_manifest_writer(self.spec(manifest_file.partition_spec_id)) as writer:
245+
for entry in entries_to_write:
246+
writer.add_entry(
247+
ManifestEntry.from_args(
248+
status=ManifestEntryStatus.EXISTING,
249+
snapshot_id=entry.snapshot_id,
250+
sequence_number=entry.sequence_number,
251+
file_sequence_number=entry.file_sequence_number,
252+
data_file=entry.data_file,
253+
)
254+
)
255+
existing_files.append(writer.to_manifest_file())
256+
257+
return existing_files
258+
259+
def _get_deleted_manifest_entries(self, manifest: ManifestFile) -> list[ManifestEntry]:
260+
"""Return the entries from the given manifest that should be marked as DELETED.
261+
262+
Subclasses override this to control which entries are selected for deletion
263+
and whether partition-level pruning is applied. The default returns no entries.
264+
"""
265+
return []
266+
267+
@cached_property
268+
def _cached_deleted_entries(self) -> list[ManifestEntry]:
269+
"""Scan the parent snapshot's manifests and collect entries to delete."""
270+
if self._parent_snapshot_id is not None:
271+
previous_snapshot = self._transaction.table_metadata.snapshot_by_id(self._parent_snapshot_id)
272+
if previous_snapshot is None:
273+
raise ValueError(f"Could not find the previous snapshot: {self._parent_snapshot_id}")
274+
275+
executor = ExecutorFactory.get_or_create()
276+
list_of_entries = executor.map(self._get_deleted_manifest_entries, previous_snapshot.manifests(self._io))
277+
return list(itertools.chain(*list_of_entries))
278+
else:
279+
return []
280+
281+
def _deleted_entries(self) -> list[ManifestEntry]:
282+
return self._cached_deleted_entries
283+
284+
def _refresh_deleted_entries_cache(self) -> None:
285+
"""Clear the cached deleted entries so they are recomputed on next access."""
286+
if "_cached_deleted_entries" in self.__dict__:
287+
del self.__dict__["_cached_deleted_entries"]
221288

222289
@abstractmethod
223290
def _existing_manifests(self) -> list[ManifestFile]: ...
@@ -773,89 +840,28 @@ class _OverwriteFiles(_SnapshotProducer["_OverwriteFiles"]):
773840

774841
def _existing_manifests(self) -> list[ManifestFile]:
775842
"""Determine if there are any existing manifest files."""
776-
existing_files = []
843+
return self._get_existing_manifests(should_use_manifest_pruning=True)
777844

845+
def _get_deleted_manifest_entries(self, manifest: ManifestFile) -> list[ManifestEntry]:
778846
manifest_evaluators: dict[int, Callable[[ManifestFile], bool]] = KeyDefaultDict(self._build_manifest_evaluator)
779-
if snapshot := self._transaction.table_metadata.snapshot_by_name(name=self._target_branch):
780-
for manifest_file in snapshot.manifests(io=self._io):
781-
# Manifest does not contain rows that match the files to delete partitions
782-
if not manifest_evaluators[manifest_file.partition_spec_id](manifest_file):
783-
existing_files.append(manifest_file)
784-
continue
785-
786-
entries_to_write: set[ManifestEntry] = set()
787-
found_deleted_entries: set[ManifestEntry] = set()
847+
if not manifest_evaluators[manifest.partition_spec_id](manifest):
848+
return []
788849

789-
for entry in manifest_file.fetch_manifest_entry(io=self._io, discard_deleted=True):
790-
if entry.data_file in self._deleted_data_files:
791-
found_deleted_entries.add(entry)
792-
else:
793-
entries_to_write.add(entry)
794-
795-
# Is the intercept the empty set?
796-
if len(found_deleted_entries) == 0:
797-
existing_files.append(manifest_file)
798-
continue
799-
800-
# Delete all files from manifest
801-
if len(entries_to_write) == 0:
802-
continue
803-
804-
# We have to rewrite the manifest file without the deleted data files
805-
with self.new_manifest_writer(self.spec(manifest_file.partition_spec_id)) as writer:
806-
for entry in entries_to_write:
807-
writer.add_entry(
808-
ManifestEntry.from_args(
809-
status=ManifestEntryStatus.EXISTING,
810-
snapshot_id=entry.snapshot_id,
811-
sequence_number=entry.sequence_number,
812-
file_sequence_number=entry.file_sequence_number,
813-
data_file=entry.data_file,
814-
)
815-
)
816-
existing_files.append(writer.to_manifest_file())
817-
818-
return existing_files
850+
return [
851+
ManifestEntry.from_args(
852+
status=ManifestEntryStatus.DELETED,
853+
snapshot_id=self._snapshot_id,
854+
sequence_number=entry.sequence_number,
855+
file_sequence_number=entry.file_sequence_number,
856+
data_file=entry.data_file,
857+
)
858+
for entry in manifest.fetch_manifest_entry(self._io, discard_deleted=True)
859+
if entry.data_file.content == DataFileContent.DATA and entry.data_file in self._deleted_data_files
860+
]
819861

820862
def _deleted_entries(self) -> list[ManifestEntry]:
821-
"""To determine if we need to record any deleted entries.
822-
823-
With a full overwrite all the entries are considered deleted.
824-
With partial overwrites we have to use the predicate to evaluate
825-
which entries are affected.
826-
"""
827-
if self._parent_snapshot_id is not None:
828-
previous_snapshot = self._transaction.table_metadata.snapshot_by_id(self._parent_snapshot_id)
829-
if previous_snapshot is None:
830-
# This should never happen since you cannot overwrite an empty table
831-
raise ValueError(f"Could not find the previous snapshot: {self._parent_snapshot_id}")
832-
833-
executor = ExecutorFactory.get_or_create()
834-
manifest_evaluators: dict[int, Callable[[ManifestFile], bool]] = KeyDefaultDict(self._build_manifest_evaluator)
835-
836-
def _get_entries(manifest: ManifestFile) -> list[ManifestEntry]:
837-
if not manifest_evaluators[manifest.partition_spec_id](manifest):
838-
return []
839-
840-
return [
841-
ManifestEntry.from_args(
842-
status=ManifestEntryStatus.DELETED,
843-
snapshot_id=self._snapshot_id,
844-
sequence_number=entry.sequence_number,
845-
file_sequence_number=entry.file_sequence_number,
846-
data_file=entry.data_file,
847-
)
848-
for entry in manifest.fetch_manifest_entry(self._io, discard_deleted=True)
849-
if entry.data_file.content == DataFileContent.DATA and entry.data_file in self._deleted_data_files
850-
]
851-
852-
list_of_entries = executor.map(_get_entries, previous_snapshot.manifests(self._io))
853-
deleted_entries = list(itertools.chain(*list_of_entries))
854-
else:
855-
deleted_entries = []
856-
863+
deleted_entries = self._cached_deleted_entries
857864
self._validate_required_deletes(deleted_entries)
858-
859865
return deleted_entries
860866

861867
def _validate_required_deletes(self, deleted_entries: list[ManifestEntry]) -> None:
@@ -876,6 +882,97 @@ def _validate_required_deletes(self, deleted_entries: list[ManifestEntry]) -> No
876882
raise ValidationException(f"Missing required files to delete: {', '.join(sorted(missing))}")
877883

878884

885+
class _RewriteFiles(_SnapshotProducer["_RewriteFiles"]):
886+
"""A snapshot producer that rewrites data files.
887+
888+
Produces a REPLACE snapshot that swaps existing data files for new ones without
889+
changing the logical contents of the table. This is the metadata-only operation
890+
used by compaction (bin-packing, sort, format migration).
891+
892+
Current scope:
893+
- Data file rewriting only (delete + add DataFiles)
894+
- Validates: files-to-delete exist, added_records <= deleted_records,
895+
no new delete files conflict with replaced data files
896+
897+
Future work (additive — no structural changes needed):
898+
- Delete-file rewriting (add _deleted_delete_files set + separate manifest handling)
899+
- dataSequenceNumber override (pin new files' seq to match replaced, for eq-delete safety)
900+
- validateFromSnapshot (expose _starting_snapshot_id setter for long-running planners)
901+
- ignoreEqualityDeletes in validation (coupled with dataSequenceNumber)
902+
"""
903+
904+
def _commit(self) -> UpdatesAndRequirements:
905+
if not self._deleted_data_files and not self._added_data_files:
906+
return (), ()
907+
908+
deleted_entries = self._deleted_entries()
909+
found_deleted_files = {entry.data_file for entry in deleted_entries}
910+
911+
if len(found_deleted_files) != len(self._deleted_data_files):
912+
raise ValidationException("Cannot commit, missing data files to be rewritten that are not in the table")
913+
914+
added_records = sum(f.record_count for f in self._added_data_files)
915+
deleted_records = sum(entry.data_file.record_count for entry in deleted_entries)
916+
917+
if added_records > deleted_records:
918+
raise ValidationException(
919+
f"Invalid replace: records added ({added_records}) exceeds records removed ({deleted_records})"
920+
)
921+
922+
return super()._commit()
923+
924+
def _get_deleted_manifest_entries(self, manifest: ManifestFile) -> list[ManifestEntry]:
925+
return [
926+
ManifestEntry.from_args(
927+
status=ManifestEntryStatus.DELETED,
928+
snapshot_id=self.snapshot_id,
929+
sequence_number=entry.sequence_number,
930+
file_sequence_number=entry.file_sequence_number,
931+
data_file=entry.data_file,
932+
)
933+
for entry in manifest.fetch_manifest_entry(self._io, discard_deleted=True)
934+
if entry.data_file.content == DataFileContent.DATA and entry.data_file in self._deleted_data_files
935+
]
936+
937+
def _existing_manifests(self) -> list[ManifestFile]:
938+
return self._get_existing_manifests(should_use_manifest_pruning=False)
939+
940+
def _validate_concurrency(self) -> None:
941+
"""Validate that concurrent changes do not conflict with this replace.
942+
943+
Unlike overwrite/delete, a replace operation only needs to validate that no new
944+
delete files have been added that would apply to the data files being replaced.
945+
Concurrent data file additions (appends) do NOT conflict with a replace because
946+
the replace only touches files it explicitly planned to rewrite.
947+
948+
This matches Java's BaseRewriteFiles.validate() which only calls
949+
validateNoNewDeletesForDataFiles, not validateAddedDataFiles or
950+
validateDeletedDataFiles.
951+
"""
952+
from pyiceberg.table.update.validate import _validate_no_new_deletes_for_data_files
953+
954+
if self._commit_window is None or self._commit_window.is_empty():
955+
return
956+
957+
catalog_head = self._commit_window.head
958+
starting_snapshot = self._commit_window.base
959+
960+
if catalog_head is None:
961+
return
962+
963+
if self._deleted_data_files:
964+
table = self._transaction._table
965+
conflict_detection_filter = self._predicate if self._predicate != AlwaysFalse() else None
966+
_validate_no_new_deletes_for_data_files(
967+
table, catalog_head, conflict_detection_filter, self._deleted_data_files, starting_snapshot
968+
)
969+
970+
def _refresh_for_retry(self) -> None:
971+
"""Reset state for a retry attempt, clearing the cached deleted entries."""
972+
super()._refresh_for_retry()
973+
self._refresh_deleted_entries_cache()
974+
975+
879976
class UpdateSnapshot:
880977
_transaction: Transaction
881978
_io: FileIO
@@ -933,6 +1030,15 @@ def delete(self) -> _DeleteFiles:
9331030
snapshot_properties=self._snapshot_properties,
9341031
)
9351032

1033+
def replace(self) -> _RewriteFiles:
1034+
return _RewriteFiles(
1035+
operation=Operation.REPLACE,
1036+
transaction=self._transaction,
1037+
io=self._io,
1038+
branch=self._branch,
1039+
snapshot_properties=self._snapshot_properties,
1040+
)
1041+
9361042

9371043
class _ManifestMergeManager(Generic[U]):
9381044
_target_size_bytes: int

0 commit comments

Comments
 (0)