This is an automated email from the ASF dual-hosted git repository.
kevinjqliu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
The following commit(s) were added to refs/heads/main by this push:
new b31b76257 fix(table): avoid committing update builders after
exceptions (#3354)
b31b76257 is described below
commit b31b7625748c42f8c35a117f8ef9612c25533bbf
Author: Minh Vu <[email protected]>
AuthorDate: Tue May 19 16:15:46 2026 +0200
fix(table): avoid committing update builders after exceptions (#3354)
## Summary
Avoid committing update builders when the body of a `with` block raises.
`UpdateTableMetadata.__exit__` currently commits unconditionally, so
user code like `with table.update_schema() as update:` can still mutate
table metadata even when an exception is raised before the block
finishes. This change makes update builders mirror
`Transaction.__exit__` and only commit on a clean exit.
## Testing
- `.venv\\Scripts\\python -m pytest
"tests/catalog/test_catalog_behaviors.py::test_update_schema_with_statement_does_not_commit_on_exception[memory]"
"tests/catalog/test_catalog_behaviors.py::test_update_schema_with_statement_does_not_commit_on_exception[sql]"
-q`
---
pyiceberg/table/update/__init__.py | 8 +++++---
tests/catalog/test_catalog_behaviors.py | 20 ++++++++++++++++++++
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/pyiceberg/table/update/__init__.py
b/pyiceberg/table/update/__init__.py
index e892b838c..64838b0bd 100644
--- a/pyiceberg/table/update/__init__.py
+++ b/pyiceberg/table/update/__init__.py
@@ -21,6 +21,7 @@ import uuid
from abc import ABC, abstractmethod
from datetime import datetime
from functools import singledispatch
+from types import TracebackType
from typing import TYPE_CHECKING, Annotated, Any, Generic, Literal, TypeVar,
cast
from pydantic import Field, field_validator, model_serializer, model_validator
@@ -71,9 +72,10 @@ class UpdateTableMetadata(ABC, Generic[U]):
def commit(self) -> None:
self._transaction._apply(*self._commit())
- def __exit__(self, _: Any, value: Any, traceback: Any) -> None:
- """Close and commit the change."""
- self.commit()
+ def __exit__(self, exctype: type[BaseException] | None, excinst:
BaseException | None, exctb: TracebackType | None) -> None:
+ """Close and commit the change if no exceptions have been raised."""
+ if exctype is None and excinst is None and exctb is None:
+ self.commit()
def __enter__(self) -> U:
"""Update the table."""
diff --git a/tests/catalog/test_catalog_behaviors.py
b/tests/catalog/test_catalog_behaviors.py
index 0a10c556e..b859e2d54 100644
--- a/tests/catalog/test_catalog_behaviors.py
+++ b/tests/catalog/test_catalog_behaviors.py
@@ -942,6 +942,26 @@ def test_add_column_with_statement(catalog: Catalog,
table_schema_simple: Schema
assert table.schema().schema_id == 2
+def test_update_schema_with_statement_does_not_commit_on_exception(
+ catalog: Catalog, table_schema_simple: Schema, random_table_identifier:
Identifier
+) -> None:
+ namespace = Catalog.namespace_from(random_table_identifier)
+ catalog.create_namespace(namespace)
+ table = catalog.create_table(random_table_identifier, table_schema_simple)
+
+ with pytest.raises(ValueError):
+ with table.update_schema() as tx:
+ tx.add_column(path="should_not_commit", field_type=IntegerType())
+ int("boom")
+
+ assert table.schema() == table_schema_simple
+ assert table.schema().schema_id == 0
+
+ reloaded = catalog.load_table(random_table_identifier)
+ assert reloaded.schema() == table_schema_simple
+ assert reloaded.schema().schema_id == 0
+
+
# Namespace tests
