DerGut commented on code in PR #862:
URL: https://github.com/apache/iceberg-rust/pull/862#discussion_r2099846902
##########
crates/catalog/sql/src/catalog.rs:
##########
@@ -769,23 +763,94 @@ impl Catalog for SqlCatalog {
Ok(())
}
- async fn update_table(&self, _commit: TableCommit) -> Result<Table> {
- Err(Error::new(
- ErrorKind::FeatureUnsupported,
- "Updating a table is not supported yet",
- ))
+ async fn update_table(&self, mut commit: TableCommit) -> Result<Table> {
+ let identifier = commit.identifier().clone();
+ if !self.table_exists(&identifier).await? {
+ return no_such_table_err(&identifier);
+ }
+
+ let requirements = commit.take_requirements();
+ let table_updates = commit.take_updates();
+
+ let table = self.load_table(&identifier).await?;
+ let mut update_table_metadata_builder =
+ TableMetadataBuilder::new_from_metadata(table.metadata().clone(),
None);
+
+ for table_update in table_updates {
+ update_table_metadata_builder =
table_update.apply(update_table_metadata_builder)?;
+ }
+
+ for table_requirement in requirements {
Review Comment:
Shouldn't the requirements be checked in a transaction (that executes the
update statement)? Otherwise a conflicting concurrent commit can update first
and we end up in a broken table state.
The table metadata that's used to validate the requirements would also need
to be loaded within the transaction.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
