This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new fdb22354ce [Cherry-pick to branch-1.3] [#13278] fix(core): clean up
missing schemas on explicit cascading drops (#13279) (#13282)
fdb22354ce is described below
commit fdb22354cead724a48986f83a300b3a15cf11a10
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 17 22:06:25 2026 +0800
[Cherry-pick to branch-1.3] [#13278] fix(core): clean up missing schemas on
explicit cascading drops (#13279) (#13282)
### What changes were proposed in this pull request?
Backport dc2545f03594c239e0884bedb58fe862c67f4247 (#13279) to
`branch-1.3`, resolving the cherry-pick conflicts.
An explicit cascading drop removes a schema registration even when the
source schema is already absent and reports success when either is
removed. A non-cascading drop preserves the registration when the source
schema is absent. Existing hierarchical ancestor cleanup is retained.
The SecretManager cleanup and its test are excluded because schema
write-through secrets are not supported on this branch.
### Why are the changes needed?
Report metadata-only cleanup accurately and retain the original fix's
distinction between explicit cascading cleanup and non-cascading drops.
Related: #13278
### Does this PR introduce _any_ user-facing change?
Yes. Metadata-only cascading cleanup returns `dropped: true`; a repeated
drop returns false. A non-cascading drop of a missing source schema
preserves its stored registration.
### How was this patch tested?
- `./gradlew spotlessApply :core:test -PskipITs -PskipDockerTests=true
-PskipWeb=true`
- Core suite: 1601 tests, 0 failures, 0 errors, 2 skipped.
- Regression tests cover source-side deletion, repeated drops, and
non-cascading preservation.
- `git diff --check`.
---------
Co-authored-by: Qi Yu <[email protected]>
---
.../catalog/SchemaOperationDispatcher.java | 27 ++++++++---------
.../catalog/TestSchemaOperationDispatcher.java | 34 ++++++++++++++++++++++
2 files changed, 46 insertions(+), 15 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 a6cd4589f0..54ea2df613 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
@@ -339,20 +339,17 @@ public class SchemaOperationDispatcher extends
OperationDispatcher implements Sc
return droppedFromCatalog;
}
- // For the unmanaged schema, it could happen that the schema:
- // 1. It's not found in the catalog (dropped directly from
underlying sources)
- // 2. It's found in the catalog but not in the store (not managed by
Gravitino)
- // 3. It's found in the catalog and the store (managed by Gravitino)
- // 4. Neither found in the catalog nor in the store.
- // In all situations, we try to delete the schema from the store,
but we don't take the
- // return value of the store operation into account. We only take
the return value of the
- // catalog into account.
- try {
- 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) {
- throw new RuntimeException(e);
+ // 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 {
+ 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) {
+ throw new RuntimeException(e);
+ }
}
SchemaEntityCleaner.deleteOrphanedSchemaEntities(
@@ -364,7 +361,7 @@ public class SchemaOperationDispatcher extends
OperationDispatcher implements Sc
catalogIdent,
c -> c.doWithSchemaOps(s -> s.schemaExists(schemaIdent)),
RuntimeException.class));
- 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 455e428c34..c452699214 100644
---
a/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
+++
b/core/src/test/java/org/apache/gravitino/catalog/TestSchemaOperationDispatcher.java
@@ -369,6 +369,40 @@ public class TestSchemaOperationDispatcher extends
TestOperationDispatcher {
RuntimeException.class, () -> dispatcher.dropSchema(schemaIdent,
false));
}
+ @Test
+ void testDropMissingSchemaPreservesStoredEntity() throws Exception {
+ reset(entityStore);
+ NameIdentifier schemaIdent = NameIdentifier.of(metalake, catalog,
"schema_renamed_out_of_band");
+ Map<String, String> props = ImmutableMap.of("k1", "v1", "k2", "v2");
+ dispatcher.createSchema(schemaIdent, "comment", props);
+
+ boolean droppedFromSource =
+ catalogManager.doWithCatalogWrapper(
+ NameIdentifier.of(metalake, catalog),
+ wrapper -> wrapper.doWithSchemaOps(ops ->
ops.dropSchema(schemaIdent, false)));
+ Assertions.assertTrue(droppedFromSource);
+
+ Assertions.assertFalse(dispatcher.dropSchema(schemaIdent, false));
+ 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
public void testDropHierarchicalSchemaCleansUpOrphanedAncestors() throws
IOException {
// Clear any spy stubs leaked from other tests sharing the static
entityStore.
