mchades commented on code in PR #12833:
URL: https://github.com/apache/gravitino/pull/12833#discussion_r4120424419
##########
core/src/main/java/org/apache/gravitino/catalog/ManagedSemanticModelOperations.java:
##########
@@ -131,15 +141,106 @@ public SemanticModel createSemanticModel(
public SemanticModel alterSemanticModel(NameIdentifier ident,
SemanticModelChange... changes)
throws NoSuchSemanticModelException, SemanticModelAlreadyExistsException,
IllegalSemanticModelException {
- // TODO: Implement in the Semantic Model list/alter/drop capability.
- throw new UnsupportedOperationException(
- "alterSemanticModel: list/alter/drop capability is not implemented");
+ boolean validateForWrite = requiresWriteValidation(changes);
+
+ try {
+ return store.update(
+ ident,
+ SemanticModelEntity.class,
+ Entity.EntityType.SEMANTIC_MODEL,
+ oldEntity -> {
+ SemanticModelEntity candidate = applyChanges(oldEntity, changes);
+ if (validateForWrite) {
+ writeValidator.accept(
+ NameIdentifier.of(candidate.namespace(), candidate.name()),
+ candidate.definition());
+ }
Review Comment:
This Core path is intentionally definition-only. During the [#12831
review](https://github.com/apache/gravitino/pull/12831#discussion_r4089149729),
catalog-backed lookups were removed from `SemanticModelOperationDispatcher`
because internal dispatchers do not carry the caller's authorization context.
`ReplaceDefinition` runs the same deterministic Core definition validator as
create, while metadata-only changes skip it. The caller-facing
`SemanticModelSourceValidator` is being added in #13512 and must be invoked by
the alter REST path in #12626 whenever the batch contains `ReplaceDefinition`.
I’ll clarify this PR’s wording from “complete write validation” to “complete
Core definition validation.” No catalog-backed validation should be added to
this Core dispatcher.
--
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]
