From 32a3b4e9deb23ff8d1e72d69adc7b16f3932dea1 Mon Sep 17 00:00:00 2001 From: Bishop Kammeraad Date: Thu, 15 Oct 2026 10:02:37 -0400 Subject: [PATCH 1/2] fix(table): reuse partition field IDs on v3 tables UpdateSpec only looked up historical partition fields when the format version was exactly 2, so v3 tables got a fresh field ID when a removed partition field was added back. Java does this for format version >= 2. --- pyiceberg/table/update/spec.py | 2 +- tests/table/test_init.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pyiceberg/table/update/spec.py b/pyiceberg/table/update/spec.py index e060d6c261..5b55d96b19 100644 --- a/pyiceberg/table/update/spec.py +++ b/pyiceberg/table/update/spec.py @@ -268,7 +268,7 @@ def _add_new_field( return PartitionSpec(*partition_fields, spec_id=new_spec_id) def _partition_field(self, transform_key: tuple[int, Transform[Any, Any]], name: str | None) -> PartitionField: - if self._transaction.table_metadata.format_version == 2: + if self._transaction.table_metadata.format_version >= 2: source_id, transform = transform_key historical_fields = [] for spec in self._transaction.table_metadata.specs().values(): diff --git a/tests/table/test_init.py b/tests/table/test_init.py index 3d160781e3..1a07ce51a5 100644 --- a/tests/table/test_init.py +++ b/tests/table/test_init.py @@ -1454,6 +1454,19 @@ def test_last_assigned_partition_id(table_v2: Table) -> None: AssertLastAssignedPartitionId(last_assigned_partition_id=1).validate(base_metadata) +@pytest.mark.parametrize("table", [lf("table_v2"), lf("table_v3")]) +def test_update_spec_reuses_removed_partition_field_id(table: Table) -> None: + txn = table.transaction() + txn.update_spec().remove_field("x").add_field("y", BucketTransform(16), "y_bucket").commit() + txn.update_spec().add_identity("x").commit() + + assert txn.table_metadata.spec().fields == ( + PartitionField(source_id=2, field_id=1001, transform=BucketTransform(16), name="y_bucket"), + PartitionField(source_id=1, field_id=1000, transform=IdentityTransform(), name="x"), + ) + assert txn.table_metadata.last_partition_id == 1001 + + def test_assert_default_spec_id(table_v2: Table) -> None: base_metadata = table_v2.metadata AssertDefaultSpecId(default_spec_id=base_metadata.default_spec_id).validate(base_metadata) From 2b28cad0c553559908a6201ca85f2bbc7974e88c Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sat, 24 Oct 2026 14:27:00 -0700 Subject: [PATCH 2/2] test: cover removing and re-adding a partition field in one update Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/table/test_init.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/table/test_init.py b/tests/table/test_init.py index 1a07ce51a5..94d6d0bc07 100644 --- a/tests/table/test_init.py +++ b/tests/table/test_init.py @@ -1467,6 +1467,17 @@ def test_update_spec_reuses_removed_partition_field_id(table: Table) -> None: assert txn.table_metadata.last_partition_id == 1001 +@pytest.mark.parametrize("table", [lf("table_v2"), lf("table_v3")]) +def test_update_spec_remove_and_add_same_field_reuses_id(table: Table) -> None: + txn = table.transaction() + txn.update_spec().remove_field("x").add_identity("x").commit() + + assert txn.table_metadata.spec().fields == ( + PartitionField(source_id=1, field_id=1000, transform=IdentityTransform(), name="x"), + ) + assert txn.table_metadata.last_partition_id == 1000 + + def test_assert_default_spec_id(table_v2: Table) -> None: base_metadata = table_v2.metadata AssertDefaultSpecId(default_spec_id=base_metadata.default_spec_id).validate(base_metadata)