This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new dc2545f035 [#13278] fix(core): clean up missing schemas on explicit
cascading drops (#13279)
dc2545f035 is described below
commit dc2545f03594c239e0884bedb58fe862c67f4247
Author: Qi Yu <[email protected]>
AuthorDate: Thu Sep 17 20:28:59 2026 +0800
[#13278] fix(core): clean up missing schemas on explicit cascading drops
(#13279)
### What changes were proposed in this pull request?
Allow an explicit cascading drop to remove an unmanaged schema's stored
registration when the source schema is already absent. Report success if
either the source schema or its registration was removed, and clean up
the schema's write-through secrets.
Preserve the existing behavior of non-cascading drops for missing
schemas.
### Why are the changes needed?
A source-side deletion currently leaves a registration that even an
explicit cascading drop cannot remove.
Fix: #13278
### Does this PR introduce _any_ user-facing change?
Yes. A metadata-only cascading schema drop returns `dropped: true`; a
repeated drop returns false. Stored schema write-through secrets are
removed with the registration.
### How was this patch tested?
- `./gradlew spotlessApply`
- `./gradlew :core:test -PskipITs` — 2,192 passed, 2 skipped.
- Regression tests cover source-side deletion, repeated drops, preserved
non-cascading behavior, and write-through secret cleanup.
---
.../catalog/SchemaOperationDispatcher.java | 15 +++----
.../catalog/TestSchemaOperationDispatcher.java | 51 ++++++++++++++++++++++
2 files changed, 58 insertions(+), 8 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
index 01da4659e4..e2fdd1fdf3 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
@@ -572,13 +572,12 @@ public class SchemaOperationDispatcher extends
OperationDispatcher implements Sc
return droppedFromCatalog;
}
- // A false result is ambiguous: the external schema may have been
renamed or dropped out
- // of band. Preserve the registration because deleting it after a
rename would lose
- // Gravitino-only metadata. A true out-of-band drop can therefore
leave a stale
- // registration that requires separate cleanup.
- if (droppedFromCatalog) {
+ // A non-cascading drop preserves a missing registration because the
source schema
+ // may have been renamed. An explicit cascading drop also removes
stale metadata.
+ boolean droppedFromStore = false;
+ if (droppedFromCatalog || cascade) {
try {
- store.delete(ident, SCHEMA, true);
+ droppedFromStore = store.delete(ident, SCHEMA, true);
} catch (NoSuchEntityException e) {
LOG.warn("The schema to be dropped does not exist in the store:
{}", ident, e);
} catch (Exception e) {
@@ -595,10 +594,10 @@ public class SchemaOperationDispatcher extends
OperationDispatcher implements Sc
catalogIdent,
c -> c.doWithSchemaOps(s -> s.schemaExists(schemaIdent)),
RuntimeException.class));
- if (droppedFromCatalog) {
+ if (droppedFromCatalog || droppedFromStore) {
secretManager.deleteSecretsFromProperties(schemaProperties);
}
- return droppedFromCatalog;
+ return droppedFromCatalog || droppedFromStore;
});
}
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
b/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
index a631e8fbbb..a04c549a4c 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
@@ -400,6 +400,57 @@ public class TestSchemaOperationDispatcher extends
TestOperationDispatcher {
Assertions.assertTrue(entityStore.exists(schemaIdent, SCHEMA));
}
+ @Test
+ void testDropSchemaRemovedFromSourceReportsMetadataCleanup() throws
Exception {
+ reset(entityStore);
+ NameIdentifier ident = NameIdentifier.of(metalake, catalog,
"externally_dropped_schema");
+ dispatcher.createSchema(ident, "comment", ImmutableMap.of("k1", "v1",
"k2", "v2"));
+ boolean droppedFromSource =
+ catalogManager.doWithCatalogWrapper(
+ NameIdentifier.of(metalake, catalog),
+ wrapper -> wrapper.doWithSchemaOps(ops -> ops.dropSchema(ident,
true)));
+ Assertions.assertTrue(droppedFromSource);
+ Assertions.assertTrue(entityStore.exists(ident, SCHEMA));
+
+ Assertions.assertTrue(dispatcher.dropSchema(ident, true));
+ Assertions.assertFalse(entityStore.exists(ident, SCHEMA));
+ Assertions.assertFalse(dispatcher.dropSchema(ident, true));
+ }
+
+ @Test
+ void testCascadingDropOfMissingSchemaDeletesStoredSecrets() throws Exception
{
+ reset(entityStore);
+ try (SecretManager secrets = memorySecretManager()) {
+ SchemaOperationDispatcher d =
+ new SchemaOperationDispatcher(catalogManager, entityStore,
idGenerator, secrets);
+ NameIdentifier ident = NameIdentifier.of(metalake, catalog,
"missing_schema_secret");
+ d.createSchema(
+ ident,
+ "comment",
+ ImmutableMap.of("k1", "v1"),
+ Map.of("k2", new SecretBinding("memory", "s3cr3t")),
+ Map.of());
+ SchemaEntity entity = entityStore.get(ident, SCHEMA, SchemaEntity.class);
+ SecretUrn urn =
+ SecretUrn.buildWriteThrough(
+ "memory",
+ Map.of(
+ SecretConstants.ATTR_ENTITY_TYPE, "schema",
+ SecretConstants.ATTR_ENTITY_ID, String.valueOf(entity.id()),
+ SecretConstants.ATTR_PROPERTY_KEY, "k2"));
+ boolean sourceDropped =
+ catalogManager.doWithCatalogWrapper(
+ NameIdentifier.of(metalake, catalog),
+ wrapper -> wrapper.doWithSchemaOps(ops -> ops.dropSchema(ident,
true)));
+ Assertions.assertTrue(sourceDropped);
+ Assertions.assertFalse(d.dropSchema(ident, false));
+ Assertions.assertEquals("s3cr3t", secrets.readSecret(urn));
+ Assertions.assertTrue(d.dropSchema(ident, true));
+ Assertions.assertFalse(entityStore.exists(ident, SCHEMA));
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
secrets.readSecret(urn));
+ }
+ }
+
@Test
public void testDropHierarchicalSchemaCleansUpOrphanedAncestors() throws
IOException {
// Clear any spy stubs leaked from other tests sharing the static
entityStore.
