[#13176] fix(core): Guard non-force parent drops inside transactions - #13499

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Verdict: ship it

Reviewed the full change on the PR head with the repository checked out (apache/gravitino at 520d9ec, diffed against origin/main = ab07853). No earlier Claude review exists on this PR, so this covers the whole change.

The core of the change is sound, and the two load-bearing assumptions hold in the code as it stands:

• CatalogMetaService.deleteCatalog line 311 takes the catalog row with selectCatalogMetaByIdForUpdate (see deleteCatalogWithVersion, CatalogMetaService.java:521-530), and SchemaMetaService.insertSchema locks the same row first (lockCatalogForSchemaCreate, SchemaMetaService.java:172-177). So the delete-then-check order really does fence a concurrent schema create.
• CatalogMetaMapper / MetalakeMetaMapper: insertCatalog locks the metalake row before inserting (CatalogMetaService.java:201-202), so the same argument holds one level up for the metalake path.

Two other things I checked and found correct, in case they come up in review:

• The MetalakeMetaService refactor preserves the cascade cleanup set exactly: the old cascade list had 30 entries, and catalogScopedCleanups (15) + metalakeScopedCleanups (15) is the same 30. SemanticModel* and View* are correctly in the catalog-scoped list, since both are namespaced under (metalake, catalog, schema).
• JDBCBackend.deleteCatalogWithAllowedSchemas writes the change log unconditionally where delete() gates on shouldRecordEntityDrop. That is equivalent here: CATALOG is in BaseEntityCache.CACHEABLE_TYPES (BaseEntityCache.java:55-66), so the predicate is always true for this entity type.
• The comment at CatalogManager.java:1483-1485 about REST reporting an unsupported operation is accurate: ExceptionHandlers.BaseExceptionHandler maps UnsupportedOperationException to Utils.unsupportedOperation (ExceptionHandlers.java:1247-1250).

One thing worth stating explicitly, because it is what makes the new UnsupportedOperationException safe: on the non-force path, a managed-storage catalog can only reach the store delete with an empty schemaEntities, because containsUserCreatedSchemas returns true for any non-empty schema list on such a catalog (CatalogManager.java:1525-1530). So the allowlist branch is only ever reached for unmanaged catalogs, where nothing destructive (physical ops.dropSchema, secret deletion) has happened yet when the transaction rolls back. The rejection is therefore side-effect free. This is subtle enough that a short comment near CatalogManager.java:1425 would help the next reader.

Tests: adequate for the paths this PR adds. The new TestCatalogMetaService / TestEntityChangeLogService cases cover both outcomes of the allowlist delete against H2, and the three CatalogManager tests cover the allowlist call, the late-schema rejection and the fail-closed store. Two gaps I would like to see closed, neither blocking:

• Nothing exercises the schemaEntities.isEmpty() -> store.delete(ident, CATALOG, false) branch (CatalogManager.java:1424) from the manager. That is the branch every managed-storage catalog now takes, and it is the first time CatalogMetaService's non-cascade catalog path is reachable from production code — before this PR CatalogManager always passed cascade = true, so that else branch (CatalogMetaService.java:396-437) was dead outside unit tests. TestCatalogMetaService.testNonCascadeDeleteRollsBackCatalogFence covers the service, but not the manager wiring.
• No test asserts the NonEmptyCatalogException message/cause for the late-schema case beyond the exception type.

I could not compile or run tests in this environment: only JDK 21 is installed and build.gradle.kts:62-64 rejects anything other than JDK 17, so every finding below comes from reading the code rather than from execution. CI results on the PR should be the source of truth for compilation and the new tests.

Review drafted with Claude, requested by @jerryshao.