syun64 commented on code in PR #265:
URL: https://github.com/apache/iceberg-python/pull/265#discussion_r1451754672
##########
pyiceberg/catalog/sql.py:
##########
@@ -268,16 +269,32 @@ def drop_table(self, identifier: Union[str, Identifier])
-> None:
identifier_tuple = self.identifier_to_tuple_without_catalog(identifier)
database_name, table_name =
self.identifier_to_database_and_table(identifier_tuple, NoSuchTableError)
with Session(self.engine) as session:
- res = session.execute(
- delete(IcebergTables).where(
- IcebergTables.catalog_name == self.name,
- IcebergTables.table_namespace == database_name,
- IcebergTables.table_name == table_name,
+ if self.engine.dialect.supports_sane_rowcount:
+ res = session.execute(
+ delete(IcebergTables).where(
+ IcebergTables.catalog_name == self.name,
+ IcebergTables.table_namespace == database_name,
+ IcebergTables.table_name == table_name,
+ )
)
- )
+ if res.rowcount < 1:
+ raise NoSuchTableError(f"Table does not exist:
{database_name}.{table_name}")
+ else:
+ try:
+ tbl = (
+ session.query(IcebergTables)
+ .with_for_update(of=IcebergTables, nowait=True)
Review Comment:
Hi @HonahX . Thank you for explaining so patiently.
> If the engine lacks rowcount support, sticking with SELECT FOR UPDATE
ensures consistent behavior with UPDATE TABLE operations, as it waits for
locks. This consistency eliminates the need to handle additional exceptions, as
any concurrent transaction will naturally lead to a NoResultFound scenario once
the table is dropped, renamed, or updated.
I agree with your analysis here, and I think that dropping **nowait** and
**skip_locked** will be best to mimic the other behavior with UPDATE TABLE
operation as closely as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
