Copilot commented on code in PR #11362:
URL: https://github.com/apache/gravitino/pull/11362#discussion_r3353289180
##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -420,6 +432,17 @@ public boolean purgeTable(NameIdentifier ident) throws
UnsupportedOperationExcep
} catch (Exception e) {
throw new RuntimeException(e);
}
+ if (droppedFromCatalog) {
+ SchemaEntityCleaner.deleteOrphanedSchemaEntities(
+ store,
+ schemaIdentifier,
+ true,
+ schemaIdent ->
+ doWithCatalog(
+ catalogIdent,
+ c -> c.doWithSchemaOps(s ->
s.schemaExists(schemaIdent)),
+ RuntimeException.class));
+ }
Review Comment:
`purgeTable` has the same `if (droppedFromCatalog)` guard around
orphaned-schema cleanup as `dropTable`. When the underlying purge returns false
because the table is already gone, Gravitino may still have orphaned schema
entities if the backend auto-dropped empty namespaces. Running
`SchemaEntityCleaner` unconditionally here is safe (it stops as soon as a
schema exists) and avoids leaving stale schema metadata.
##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -366,6 +367,17 @@ public boolean dropTable(NameIdentifier ident) {
} catch (Exception e) {
throw new RuntimeException(e);
}
+ if (droppedFromCatalog) {
+ SchemaEntityCleaner.deleteOrphanedSchemaEntities(
+ store,
+ schemaIdentifier,
+ true,
+ schemaIdent ->
+ doWithCatalog(
+ catalogIdent,
+ c -> c.doWithSchemaOps(s ->
s.schemaExists(schemaIdent)),
+ RuntimeException.class));
+ }
Review Comment:
The orphaned-schema cleanup is only triggered when `droppedFromCatalog` is
true. If a table is already missing in the underlying catalog (e.g., dropped
out-of-band) and the backend auto-removed empty namespaces, this path will
delete the table entity from the store but leave orphaned schema entities
behind. Since `SchemaEntityCleaner` is best-effort and uses `schemaExists`
checks, it can safely run even when `dropTable` returns false.
##########
core/src/main/java/org/apache/gravitino/catalog/ViewOperationDispatcher.java:
##########
@@ -292,6 +293,17 @@ public boolean dropView(NameIdentifier ident) {
} catch (Exception e) {
throw new RuntimeException(e);
}
+ if (droppedFromCatalog) {
+ SchemaEntityCleaner.deleteOrphanedSchemaEntities(
+ store,
+ schemaIdentifier,
+ true,
+ schemaIdent ->
+ doWithCatalog(
+ catalogIdent,
+ c -> c.doWithSchemaOps(s ->
s.schemaExists(schemaIdent)),
+ RuntimeException.class));
+ }
Review Comment:
The orphaned-schema cleanup is currently skipped when `dropView` returns
false. If the view was dropped outside Gravitino and the catalog auto-removed
empty namespaces, this will remove only the view entity while leaving orphaned
schema entities in the store. Because `SchemaEntityCleaner` is best-effort and
checks `schemaExists`, it should run regardless of `droppedFromCatalog`.
--
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]
