liurenjie1024 commented on code in PR #1894:
URL: https://github.com/apache/iceberg-rust/pull/1894#discussion_r2638463379
##########
crates/catalog/hms/src/catalog.rs:
##########
@@ -603,10 +705,94 @@ impl Catalog for HmsCatalog {
))
}
- async fn update_table(&self, _commit: TableCommit) -> Result<Table> {
- Err(Error::new(
- ErrorKind::FeatureUnsupported,
- "Updating a table is not supported yet",
- ))
+ /// Updates an existing table by applying a commit operation.
+ ///
+ /// This method supports two update strategies depending on the catalog
configuration:
+ ///
+ /// **Optimistic Locking** (when `hive_locks_disabled` is set):
+ /// - Retrieves the current table state from HMS without acquiring locks
+ /// - Constructs an `EnvironmentContext` with the expected metadata
location
+ /// - Uses `alter_table_with_environment_context` to perform an atomic
+ /// compare-and-swap operation.
+ /// - HMS will reject the update if the metadata location has changed,
+ /// indicating a concurrent modification
+ ///
+ /// **Traditional Locking** (default):
+ /// - Acquires an exclusive HMS lock on the table before making changes
+ /// - Retrieves the current table state
+ /// - Applies the commit and writes new metadata
+ /// - Updates the table in HMS using `alter_table`
+ /// - Releases the lock after the operation completes
+ ///
+ /// # Returns
+ /// A `Result` wrapping the updated `Table` object with new metadata.
+ ///
+ /// # Errors
+ /// This function may return an error in several scenarios:
+ /// - Failure to validate the namespace or table identifier
+ /// - Inability to acquire a lock (traditional locking mode)
+ /// - Failure to retrieve the table from HMS
+ /// - Errors reading or writing table metadata
+ /// - HMS rejects the update due to concurrent modification (optimistic
locking)
+ /// - Errors from the underlying Thrift communication with HMS
+ async fn update_table(&self, commit: TableCommit) -> Result<Table> {
+ let ident = commit.identifier().clone();
+ let db_name = validate_namespace(ident.namespace())?;
+ let tbl_name = ident.name.clone();
+
+ if self.config.props.contains_key(HMS_HIVE_LOCKS_DISABLED) {
Review Comment:
I'm not a hive expert, but I would suggest to start with pessimistic version
only first.
--
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]
