mchades commented on code in PR #12503:
URL: https://github.com/apache/gravitino/pull/12503#discussion_r3818538424
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/ViewMetaService.java:
##########
@@ -223,6 +224,11 @@ private boolean deleteView(Long viewId) {
mapper ->
mapper.softDeleteTagMetadataObjectRelsByMetadataObject(
viewId, MetadataObject.Type.VIEW.name()));
+ SessionUtils.doWithoutCommit(
+ PolicyMetadataObjectRelMapper.class,
+ mapper ->
+ mapper.softDeletePolicyMetadataObjectRelsByMetadataObject(
Review Comment:
Direct VIEW/FUNCTION drops are handled, but cascade deletion bypasses these
methods. Catalog and schema cascade deletion use
`softDeletePolicyMetadataObjectRelsByCatalogId` /
`softDeletePolicyMetadataObjectRelsBySchemaIds`, while both the H2/MySQL and
PostgreSQL SQL providers still omit `VIEW` and `FUNCTION`. This leaves active
policy relations after a catalog or schema is dropped. Please extend both
providers and add cascade-deletion coverage for both object types.
##########
core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java:
##########
@@ -184,6 +192,8 @@ public static void setUp() throws IllegalAccessException,
IOException {
.build();
entityStore.put(table, false /* overwritten */);
when(tableDispatcher.tableExists(any())).thenReturn(true);
+ when(viewDispatcher.viewExists(any())).thenReturn(true);
Review Comment:
Mocking `viewExists()` and `functionExists()` is not sufficient because the
policy relation path still resolves the metadata-object ID from the relational
store. On the current head, this test fails at the first VIEW association with
`NoSuchMetadataObjectException` because no `ViewEntity` was persisted; FUNCTION
will follow the same path. Please persist both entities in the fixture, as
`TestTagManager` does.
##########
core/src/main/java/org/apache/gravitino/policy/PolicyManager.java:
##########
@@ -63,7 +63,9 @@ public class PolicyManager implements PolicyDispatcher {
MetadataObject.Type.TABLE,
MetadataObject.Type.FILESET,
MetadataObject.Type.TOPIC,
- MetadataObject.Type.MODEL);
+ MetadataObject.Type.MODEL,
+ MetadataObject.Type.VIEW,
Review Comment:
Please update the public policy contract together with this change.
`PolicyContentBase.supportedObjectTypes` and the returned `MetadataObject` enum
in `docs/open-api/policies.yaml` still omit `VIEW` and `FUNCTION`, while
`docs/policies.md` says views and functions cannot carry policies. Otherwise
generated clients and the published documentation will disagree with the
runtime behavior. Please update them and run `./gradlew :docs:build`.
--
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]
