Copilot commented on code in PR #12833:
URL: https://github.com/apache/gravitino/pull/12833#discussion_r4119879755
##########
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:
The replacement path only invokes `SemanticModelValidator`, whose contract
explicitly performs no catalog I/O and does not check source existence, source
columns, or authorization. As a result, `replaceDefinition` can persist
references to unavailable or inaccessible external datasets even though this is
the path described as doing complete write validation; route definition
replacements through the source-aware validator, while keeping metadata-only
changes on the non-resolving path.
--
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]
