yuqi1129 commented on code in PR #11074:
URL: https://github.com/apache/gravitino/pull/11074#discussion_r3256092573
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/MetadataObjectService.java:
##########
@@ -96,6 +96,14 @@ public class MetadataObjectService {
MetadataObjectService::getJobTemplateObjectsFullName)
.build();
+ static final Map<MetadataObject.Type, BasePOStorageOps<?, ?>>
TYPE_TO_STORAGE_OPS_MAP =
+ ImmutableMap.<MetadataObject.Type, BasePOStorageOps<?, ?>>builder()
+ .put(MetadataObject.Type.SCHEMA, new SchemaPOStorageOps())
Review Comment:
These storage ops are not wrapped by `HierarchicalConversionPOStorageOps`,
so `getSchemaObjectsFullName` reads `schema_meta.schema_name` in physical form.
For a nested schema stored as `ns_a\u0001ns_b`, this method returns
`catalog.ns_a\u0001ns_b` instead of the logical/API name `catalog.ns_a:ns_b`;
table/view/function full names inherit the same physical schema segment through
`getSchemaObjectsFullName`.
This leaks the internal separator into metadata-object names used by
owner/tag/policy/auth paths. Please apply the same physical-to-logical
conversion here, or use wrapped ops for SCHEMA/TABLE/VIEW/FUNCTION in
`TYPE_TO_STORAGE_OPS_MAP`, and add coverage for
`MetadataObjectService.getSchemaObjectsFullName` / table full names with nested
schemas.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java:
##########
@@ -230,11 +198,7 @@ public void insertSchema(SchemaEntity schemaEntity,
boolean overwrite) throws IO
SchemaPO leafPO =
POConverters.initializeSchemaPOWithVersion(leafRow, leafBuilder);
Review Comment:
This PR now creates separate ancestor schema rows for nested schemas, but
`deleteSchema` still only checks and deletes objects whose `schema_id` equals
the selected row. Deleting `ns_a` therefore does not consider descendant schema
rows such as `ns_a:ns_b` / `ns_a:ns_b:leaf`, because those are separate rows
under the same catalog with different schema ids.
As a result, non-cascade delete can remove an ancestor while child schemas
still exist, and cascade delete removes only the ancestor's own objects,
leaving descendant schema rows and their tables/filesets/etc. behind. The
delete path needs to include descendant schemas by logical/physical prefix, or
reject deleting a schema that has nested children unless cascade handles the
whole subtree. Please add tests for deleting `ns_a` when `ns_a:ns_b` exists,
both cascade=false and cascade=true.
--
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]
