JingsongLi commented on code in PR #8208:
URL: https://github.com/apache/paimon/pull/8208#discussion_r3460450160


##########
paimon-core/src/main/java/org/apache/paimon/jdbc/JdbcCatalog.java:
##########
@@ -370,6 +498,57 @@ protected void createTableImpl(Identifier identifier, 
Schema schema) {
         }
     }
 
+    private boolean validateTableNotExists(Identifier identifier, boolean 
ignoreIfExists)
+            throws TableAlreadyExistException {
+        if (JdbcUtils.tableExists(
+                        connections,
+                        catalogKey,
+                        identifier.getDatabaseName(),
+                        identifier.getObjectName())
+                || JdbcUtils.viewExists(
+                        connections,
+                        catalogKey,
+                        identifier.getDatabaseName(),
+                        identifier.getObjectName())) {
+            if (ignoreIfExists) {
+                return false;
+            }
+            throw new TableAlreadyExistException(identifier);
+        }
+        return true;
+    }
+
+    @Override
+    public void renameTable(Identifier fromTable, Identifier toTable, boolean 
ignoreIfNotExists)
+            throws TableNotExistException, TableAlreadyExistException {
+        checkNotBranch(fromTable, "renameTable");
+        checkNotBranch(toTable, "renameTable");
+        checkNotSystemTable(fromTable, "renameTable");
+        checkNotSystemTable(toTable, "renameTable");
+
+        try {
+            getTable(fromTable);
+        } catch (TableNotExistException e) {
+            if (ignoreIfNotExists) {
+                return;
+            }
+            throw new TableNotExistException(fromTable);
+        }
+
+        try {
+            getTable(toTable);
+            throw new TableAlreadyExistException(toTable);
+        } catch (TableNotExistException ignored) {
+        }
+
+        if (JdbcUtils.viewExists(

Review Comment:
   This view-name check is still not atomic with the table rename. `createView` 
and `renameView` now take the catalog lock for the target identifier, but 
`renameTable` does not take any lock around this check plus `updateTable(...)`; 
since tables and views are stored in separate JDBC tables, a concurrent 
`renameTable(a -> x)` and `createView(x)`/`renameView(... -> x)` can both pass 
their checks and both commit, leaving a table and a view with the same 
identifier. Please wrap the table rename validation and metadata update with 
the same deterministic source/target locks (and add a concurrent rename-vs-view 
test), or enforce the shared object name with a database constraint.



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