From 02cac46339e20804c46f6b8e3fc475c23fa004b8 Mon Sep 17 00:00:00 2001 From: ASRodrigo1 Date: Sat, 7 Nov 2026 21:49:05 -0400 Subject: [PATCH] Share unchanged models in update_table_metadata instead of deep-copying update_table_metadata ended in model_copy(deep=True), so every call copied the whole table history (snapshots, logs, schemas) and its cost grew with the number of snapshots the table keeps. Transaction.table_metadata calls it on every read and catalogs call it once more on commit, which made one append to a table with 10,000 snapshots take seconds. Copy only the list and dict fields of the metadata and share the frozen models they hold, like TableMetadata.Builder in the Java implementation. The base metadata stays isolated from a list append or dict assignment on the result. Closes #4294 Co-Authored-By: Claude Opus 5.5 --- pyiceberg/table/update/__init__.py | 11 ++++++++++- tests/table/test_init.py | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/pyiceberg/table/update/__init__.py b/pyiceberg/table/update/__init__.py index 755e519116..53aa020489 100644 --- a/pyiceberg/table/update/__init__.py +++ b/pyiceberg/table/update/__init__.py @@ -763,7 +763,16 @@ def update_table_metadata( if enforce_validation: return TableMetadataUtil.parse_obj(new_metadata.model_dump()) else: - return new_metadata.model_copy(deep=True) + return _copy_containers(new_metadata) + + +def _copy_containers(metadata: TableMetadata) -> TableMetadata: + """Copy the list and dict fields of the metadata and share the frozen models they hold. + + The frozen config cannot prevent a list append or a dict assignment on a field, so copying the + containers keeps the base metadata isolated without deep-copying the whole table history. + """ + return metadata.model_copy(update={name: value.copy() for name, value in metadata if isinstance(value, (list, dict))}) def _update_table_metadata_log(base_metadata: TableMetadata, metadata_location: str, last_updated_ms: int) -> TableMetadata: diff --git a/tests/table/test_init.py b/tests/table/test_init.py index db6dc00e39..4cff2ec3f1 100644 --- a/tests/table/test_init.py +++ b/tests/table/test_init.py @@ -1411,6 +1411,20 @@ def test_metadata_isolation_from_illegal_updates(table_v1: Table) -> None: assert len(base_metadata.partition_specs) == 1 +def test_update_metadata_shares_unchanged_models(table_v2: Table) -> None: + base_metadata = table_v2.metadata + new_metadata = update_table_metadata(base_metadata, (SetPropertiesUpdate(updates={"test_a": "test_a"}),)) + + # The containers are copied, so a list append or dict assignment cannot reach the base metadata + assert new_metadata.snapshots is not base_metadata.snapshots + assert new_metadata.refs is not base_metadata.refs + assert new_metadata.properties is not base_metadata.properties + # The frozen models they hold are shared, so the cost does not grow with the table history + assert all(new is base for new, base in zip(new_metadata.snapshots, base_metadata.snapshots, strict=True)) + assert all(new is base for new, base in zip(new_metadata.schemas, base_metadata.schemas, strict=True)) + assert "test_a" not in base_metadata.properties + + def test_generate_snapshot_id(table_v2: Table) -> None: assert isinstance(_generate_snapshot_id(), int) assert isinstance(table_v2.metadata.new_snapshot_id(), int)