mchades commented on code in PR #12833:
URL: https://github.com/apache/gravitino/pull/12833#discussion_r4121923405


##########
core/src/main/java/org/apache/gravitino/catalog/SemanticModelOperationDispatcher.java:
##########
@@ -104,6 +104,9 @@ public SemanticModel createSemanticModel(
   public SemanticModel alterSemanticModel(NameIdentifier ident, 
SemanticModelChange... changes)
       throws NoSuchSemanticModelException, SemanticModelAlreadyExistsException,
           IllegalSemanticModelException {
+    if (changes == null || changes.length == 0) {
+      throw new IllegalSemanticModelException("At least one Semantic Model 
change is required");
+    }

Review Comment:
   This is intentional. In #12565, all `TreeLock` wrappers were removed from 
`SemanticModelOperationDispatcher` following the earlier review decision that 
this fully Gravitino-managed entity should rely on database transactions and 
concurrency controls instead.
   
   The persistence path now uses strict create semantics, parent-schema row 
locking for child writes, version-CAS/OCC, and transactional version and 
cascade cleanup. This also follows the direction tracked in #10238 and design 
PR #12157: correctness moves into the shared database rather than extending the 
per-JVM `TreeLock` to new managed operations. Cross-node read consistency is 
tracked separately in #13181.
   
   Reintroducing `TreeLock` here would not address HA races, so no change is 
needed in this PR.



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