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]