The idea is to make the code simpler if we only want to evolve schema/spec/...
i.e.

with table.update_schema() as update:
    update.add_column("some_field", IntegerType(), "doc")

instead of another with..transaction wrapper

with table.transaction() as transaction:
    with transaction.update_schema() as update_schema:
        update.add_column("some_other_field", IntegerType(), "doc")

....

What about moving the autocommit logic out of Transaction and into the class that uses it instead?

For example, UpdateSchema can implement __enter__ and __exit__ to commit the transaction automatically?

Currently, these classes use autocommit=True:

• ManageSnapshots
• UpdateSchema
• UpdateSpec

Just raise another possibility for discussion. We could also modify the _transaction._autocommit when calling UpdateSchema.__enter__ method, which indicates multiple updates is coming and we would like to avoid the autocommit. Something like:

class UpdateSchema(UpdateTableMetadata["UpdateSchema"]):
    ...
    def __enter__(self):
        self._transaction._autocommit = False
        return self
    ...

The reason behind my head is, to my understanding, autocommit is still somewhat a related concept with transaction.

Adding an example for the issue:

from pyiceberg.catalog.sql import SqlCatalog
from pyiceberg.schema import Schema
from pyiceberg.types import IntegerType, NestedField, StringType

WAREHOUSE_PATH = "/tmp/warehouse"
catalog = SqlCatalog(
    "default",
    uri=f"sqlite:///{WAREHOUSE_PATH}/pyiceberg_catalog.db", warehouse=f"file://{WAREHOUSE_PATH}",
)
catalog.create_namespace_if_not_exists("default")

try:
    catalog.drop_table("default.test")
except:
    pass

schema = Schema(
    NestedField(1, "field1", StringType(), required=False)
)

table = catalog.create_table("default.test", schema)

with table.update_schema() as update:
    update.add_column("field_should_not_exist", IntegerType())  # <--- This field should not be committed
    raise Exception("Error!")

The reason behind my head is, to my understanding, autocommit is still somewhat a related concept with transaction.

I think its fair to ask if we consider UpdateSchema to be a transaction. If so, we cannot allow autocommit since that'll break the atomicity of the transaction.

We could also modify the _transaction._autocommit when calling UpdateSchema.enter method, which indicates multiple updates is coming and we would like to avoid the autocommit. Something like:

I feel like in this case, we can just get rid of autocommit entirely

cc @Fokko / @HonahX from the original conversation