apache / apache/gravitino

[Improvement] Fix unmanaged purgeTable result when store entity is missing

Open
#10,274 1 comment 0 reactions 1 assignee Claimed by @pythaac View on GitHub
good first issue improvement
Dominant language
Java
Stars
3.2k
Forks
935
Avg merge
1d 16h
Merged PRs (30d)
298

Description

### What would you like to be improved?

TableOperationDispatcher.purgeTable for unmanaged tables returns false when store.delete(ident, TABLE) throws NoSuchEntityException, even if the catalog purge succeeded. This contradicts the method’s own comment that store deletion outcome should not affect the returned result, and only the catalog result should be used. As a result, callers can receive false negatives (DropResponse(false)) for successful catalog purges.

### How should we improve?

Align purgeTable with dropTable behavior: in the unmanaged branch, catch NoSuchEntityException, log a warning, and continue returning droppedFromCatalog instead of returning false. Keep throwing on other unexpected exceptions.

Here a test to help:
```

@Test
public void testPurgeUnmanagedTableWithMissingStoreEntityShouldUseCatalogResult()
throws IOException {
CatalogManager mockedCatalogManager = mock(CatalogManager.class);
EntityStore mockedStore = mock(EntityStore.class);
IdGenerator mockedIdGenerator = mock(IdGenerator.class);
CatalogManager.CatalogWrapper mockedCatalogWrapper = mock(CatalogManager.CatalogWrapper.class);
org.apache.gravitino.connector.capability.Capability mockedCapability =
mock(org.apache.gravitino.connector.capability.Capability.class);
TableOperationDispatcher dispatcherUnderTest =
new TableOperationDispatcher(mockedCatalogManager, mockedStore, mockedIdGenerator);

NameIdentifier tableIdent = NameIdentifier.of(metalake, catalog, "schema72", "table32");
NameIdentifier catalogIdent = NameIdentifier.of(metalake, catalog);
doReturn(mockedCatalogWrapper).when(mockedCatalogManager).loadCatalogAndWrap(catalogIdent);
doReturn(mockedCapability).when(mockedCatalogWrapper).capabilities();
doReturn(CapabilityResult.unsupported("unmanaged table"))
.when(mockedCapability)
.managedStorage(org.apache.gravitino.connector.capability.Capability.Scope.TABLE);
doReturn(true).when(mockedCatalogWrapper).doWithTableOps(any());
doThrow(new NoSuchEntityException("missing store entity"))
.when(mockedStore)
.delete(tableIdent, TABLE);

Assertions.assertTrue(dispatcherUnderTest.purgeTable(tableIdent));
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.