jerryshao commented on code in PR #8635:
URL: https://github.com/apache/gravitino/pull/8635#discussion_r2366609138


##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -199,12 +200,27 @@ public Table alterTable(NameIdentifier ident, 
TableChange... changes)
       throws NoSuchTableException, IllegalArgumentException {
     validateAlterProperties(ident, 
HasPropertyMetadata::tablePropertiesMetadata, changes);
 
-    // Check if there exist TableChange.RenameTable in the changes, if so, we 
need to TreeLock of
-    // write on the new table name, or use the read lock on the table instead.
-    boolean containsRenameTable =
-        Arrays.stream(changes).anyMatch(c -> c instanceof 
TableChange.RenameTable);
-    NameIdentifier nameIdentifierForLock =
-        containsRenameTable ? NameIdentifier.of(ident.namespace().levels()) : 
ident;
+    // use the read lock on the table if there does not exist 
TableChange.RenameTable in the
+    // changes, or:
+    // 1. if the TableChange.RenameTable change the schema name, we need to 
acquire write lock on
+    // the catalog,
+    //    because the treeLock tool currently does not support acquiring write 
locks on two tables
+    // at the same time.
+    // 2. if the TableChange.RenameTable only change the table name, we need 
to acquire write lock
+    // to on the schema.
+    NameIdentifier nameIdentifierForLock = ident;
+    String schemaName = ident.namespace().level(2);
+    for (TableChange change : changes) {
+      if (change instanceof TableChange.RenameTable) {
+        TableChange.RenameTable rename = (TableChange.RenameTable) change;
+        if (rename.getNewSchemaName().isPresent()
+            && !rename.getNewSchemaName().get().equals(schemaName)) {
+          nameIdentifierForLock = getCatalogIdentifier(ident);
+          break;
+        }
+        nameIdentifierForLock = getSchemaIdentifier(ident);
+      }
+    }

Review Comment:
   I saw it always uses the write lock, which is different from the comment 
mentioned above "use the read lock on the table if there does not exist 
TableChange.RenameTable in the changes".



-- 
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]

Reply via email to