This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 1d365c7cd6 [#9954] fix(lakehouse-generic): Enhance purgeTable logic to
handle external tables and missing locations. (#9967)
1d365c7cd6 is described below
commit 1d365c7cd6c48844dcf7c5ceac2f5c794aab2124
Author: Qi Yu <[email protected]>
AuthorDate: Fri Jun 5 22:08:51 2026 +0800
[#9954] fix(lakehouse-generic): Enhance purgeTable logic to handle external
tables and missing locations. (#9967)
### What changes were proposed in this pull request?
This pull request improves the robustness of table purge and drop
operations in the Lance catalog by handling cases where the underlying
data location is missing or was never created. It also adds
comprehensive integration tests to verify these scenarios and updates
the REST service integration tests to align with the new behavior.
**Enhancements to Table Purge/Drop Logic:**
* Updated `LanceTableOperations.purgeTable` to check if the dataset
location exists before attempting deletion, preventing errors when the
data is already missing or was never created. The method now logs a
warning and skips deletion if the dataset cannot be opened.
* Added import for `ReadOptions` in `LanceTableOperations` to support
the new dataset existence check.
**Integration and REST API Test Improvements:**
* Added new integration tests in `CatalogGenericCatalogLanceIT` to
verify that purging tables works correctly when the data location is
missing, never created, or for managed tables with auto-generated
locations.
* Updated REST service integration test `LanceRESTServiceIT` to assert
that dropping a table does not throw an exception when the location is
missing, and added a new test to cover dropping tables with missing or
never-created locations.
### Why are the changes needed?
It's a bug.
Fix: #9954
### Does this PR introduce _any_ user-facing change?
N/A.
### How was this patch tested?
ITs.
---------
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
---
.../lakehouse/lance/LanceTableOperations.java | 58 +++++++++++-----
.../test/CatalogGenericCatalogLanceIT.java | 79 ++++++++++++++++++++++
.../lance/integration/test/LanceRESTServiceIT.java | 51 +++++++++++++-
3 files changed, 171 insertions(+), 17 deletions(-)
diff --git
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
index 9edec20a23..89c99d5365 100644
---
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
+++
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java
@@ -23,6 +23,7 @@ import static
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_TABLE
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
@@ -203,17 +204,26 @@ public class LanceTableOperations extends
ManagedTableOperations {
public boolean purgeTable(NameIdentifier ident) {
try {
Table table = loadTable(ident);
- String location = table.properties().get(Table.PROPERTY_LOCATION);
+ boolean external =
+ Optional.ofNullable(table.properties().get(Table.PROPERTY_EXTERNAL))
+ .map(Boolean::parseBoolean)
+ .orElse(false);
+ // NOTE: the two steps below (metadata removal and dataset deletion) are
NOT atomic.
+ // If the process crashes between them, the entity-store record will be
gone but the
+ // Lance dataset will still exist on storage (orphaned data), or
vice-versa. There is
+ // currently no recovery mechanism for this window.
boolean purged = super.purgeTable(ident);
+ // If the table is a managed table, super.purgeTable will call dropTable
to remove the
+ // underlying Lance dataset, so we don't need to do anything here.
+ if (!external) {
+ return purged;
+ }
+
// If the table metadata is purged successfully, we can delete the Lance
dataset.
// Otherwise, we should not delete the dataset.
if (purged) {
- // Delete the Lance dataset at the location
- Dataset.drop(
- location,
- LancePropertiesUtils.resolveLanceStorageOptions(catalogProperties,
table.properties()));
- LOG.info("Deleted Lance dataset at location {}", location);
+ dropLanceDataset(table);
}
return purged;
@@ -234,22 +244,19 @@ public class LanceTableOperations extends
ManagedTableOperations {
.map(Boolean::parseBoolean)
.orElse(false);
+ // NOTE: the two steps below (metadata removal and dataset deletion) are
NOT atomic.
+ // If the process crashes between them, the entity-store record will be
gone but the
+ // Lance dataset will still exist on storage (orphaned data), or
vice-versa. There is
+ // currently no recovery mechanism for this window.
boolean dropped = super.dropTable(ident);
if (external) {
return dropped;
}
- // If the table metadata is dropped successfully, and the table is not
external, we can delete
- // the
- // Lance dataset. Otherwise, we should not delete the dataset.
+ // If the table metadata is dropped successfully, and the table is not
external, we can
+ // delete the Lance dataset. Otherwise, we should not delete the dataset.
if (dropped) {
- String location = table.properties().get(Table.PROPERTY_LOCATION);
-
- // Delete the Lance dataset at the location
- Dataset.drop(
- location,
- LancePropertiesUtils.resolveLanceStorageOptions(catalogProperties,
table.properties()));
- LOG.info("Deleted Lance dataset at location {}", location);
+ dropLanceDataset(table);
}
return dropped;
@@ -261,6 +268,25 @@ public class LanceTableOperations extends
ManagedTableOperations {
}
}
+ private void dropLanceDataset(Table table) {
+ String location = table.properties().get(Table.PROPERTY_LOCATION);
+ Map<String, String> resolvedStorageOptions =
+ LancePropertiesUtils.resolveLanceStorageOptions(catalogProperties,
table.properties());
+ try {
+ Dataset.drop(location, resolvedStorageOptions);
+ LOG.info("Deleted Lance dataset at location {}", location);
+ } catch (Exception e) {
+ // Dataset.drop (native) throws IOException with "Not found:" when path
doesn't exist.
+ if (e instanceof IOException
+ && e.getMessage() != null
+ && e.getMessage().contains("Not found:")) {
+ LOG.warn("Lance dataset at {} was already deleted, skipping.",
location);
+ } else {
+ throw new RuntimeException("Failed to delete Lance dataset at " +
location, e);
+ }
+ }
+ }
+
// Package-private for testing
Table createTableInternal(
NameIdentifier ident,
diff --git
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
index d7b2b5f218..f2dc40907a 100644
---
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
+++
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
@@ -153,6 +153,85 @@ public class CatalogGenericCatalogLanceIT extends BaseIT {
createSchema();
}
+ @Test
+ public void testPurgeTableWhenLocationMissing() throws IOException {
+ Column[] columns = createColumns();
+ String location = tempDirectory + "/" +
GravitinoITUtils.genRandomName(TABLE_PREFIX) + "/";
+ NameIdentifier identifier =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName(TABLE_PREFIX));
+ Map<String, String> properties = createProperties();
+ properties.put(Table.PROPERTY_TABLE_FORMAT, LANCE_TABLE_FORMAT);
+ properties.put(Table.PROPERTY_EXTERNAL, "true");
+ properties.put(Table.PROPERTY_LOCATION, location);
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ identifier, columns, TABLE_COMMENT, properties,
Transforms.EMPTY_TRANSFORM, null, null);
+ Assertions.assertTrue(new File(location).exists());
+
+ // Simulate missing data before purge
+ FileUtils.deleteDirectory(new File(location));
+ Assertions.assertFalse(new File(location).exists());
+
+ Assertions.assertDoesNotThrow(() ->
catalog.asTableCatalog().purgeTable(identifier));
+ Assertions.assertFalse(catalog.asTableCatalog().tableExists(identifier));
+ }
+
+ @Test
+ public void testPurgeExternalTableWithEmptyDirectory() {
+ Column[] columns = createColumns();
+ String location = tempDirectory + "/" +
GravitinoITUtils.genRandomName(TABLE_PREFIX) + "/";
+ NameIdentifier identifier =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName(TABLE_PREFIX));
+ Map<String, String> properties = createProperties();
+ properties.put(Table.PROPERTY_TABLE_FORMAT, LANCE_TABLE_FORMAT);
+ properties.put(Table.PROPERTY_EXTERNAL, "true");
+ properties.put(Table.PROPERTY_LOCATION, location);
+
+ catalog
+ .asTableCatalog()
+ .createTable(
+ identifier, columns, TABLE_COMMENT, properties,
Transforms.EMPTY_TRANSFORM, null, null);
+ // The directory is created but contains no Lance dataset files
+ Assertions.assertTrue(new File(location).exists());
+
+ Assertions.assertDoesNotThrow(() ->
catalog.asTableCatalog().purgeTable(identifier));
+ Assertions.assertFalse(catalog.asTableCatalog().tableExists(identifier));
+ }
+
+ @Test
+ public void testPurgeManagedTableWithAutoLocation() {
+ Column[] columns = createColumns();
+ NameIdentifier identifier =
+ NameIdentifier.of(schemaName,
GravitinoITUtils.genRandomName(TABLE_PREFIX));
+ String location = tempDirectory + "/" +
GravitinoITUtils.genRandomName(TABLE_PREFIX) + "/";
+
+ Map<String, String> properties = createProperties();
+ properties.put(Table.PROPERTY_TABLE_FORMAT, LANCE_TABLE_FORMAT);
+ properties.put(Table.PROPERTY_EXTERNAL, "false");
+ properties.put(Table.PROPERTY_LOCATION, location);
+
+ Table table =
+ catalog
+ .asTableCatalog()
+ .createTable(
+ identifier,
+ columns,
+ TABLE_COMMENT,
+ properties,
+ Transforms.EMPTY_TRANSFORM,
+ null,
+ null);
+ String resolvedLocation = table.properties().get(Table.PROPERTY_LOCATION);
+ Assertions.assertNotNull(resolvedLocation);
+ Assertions.assertTrue(new File(resolvedLocation).exists());
+
+ Assertions.assertDoesNotThrow(() ->
catalog.asTableCatalog().purgeTable(identifier));
+ Assertions.assertFalse(catalog.asTableCatalog().tableExists(identifier));
+ Assertions.assertFalse(new File(resolvedLocation).exists());
+ }
+
@Test
public void testCrateEmptyTable() {
String emptyTableName = GravitinoITUtils.genRandomName(TABLE_PREFIX);
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
index 40798407e4..061e61250b 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
@@ -882,7 +882,7 @@ public class LanceRESTServiceIT extends BaseIT {
// Drop the table
DropTableRequest dropTableRequest = new DropTableRequest();
dropTableRequest.setId(ids);
- Assertions.assertThrows(Exception.class, () ->
ns.dropTable(dropTableRequest));
+ Assertions.assertDoesNotThrow(() -> ns.dropTable(dropTableRequest));
// Describe the dropped table should fail
DescribeTableRequest describeTableRequest = new DescribeTableRequest();
@@ -1014,6 +1014,55 @@ public class LanceRESTServiceIT extends BaseIT {
return new TableApi(apiClient);
}
+ @Test
+ void testDropTableWhenLocationMissingOrNotCreated() {
+ catalog = createCatalog(CATALOG_NAME);
+ createSchema();
+
+ // Create table via register with non-existent location
+ List<String> registerIds = List.of(CATALOG_NAME, SCHEMA_NAME,
"register_missing_location");
+ String missingLocation = tempDir + "/" + "register_missing_location/";
+ RegisterTableRequest registerTableRequest = new RegisterTableRequest();
+ registerTableRequest.setId(registerIds);
+ registerTableRequest.setLocation(missingLocation);
+ registerTableRequest.setMode("create");
+ RegisterTableResponse registerTableResponse =
+ Assertions.assertDoesNotThrow(() ->
ns.registerTable(registerTableRequest));
+ Assertions.assertEquals(missingLocation,
registerTableResponse.getLocation());
+ Assertions.assertFalse(new File(missingLocation).exists());
+
+ DropTableRequest dropRegisteredTable = new DropTableRequest();
+ dropRegisteredTable.setId(registerIds);
+ Assertions.assertDoesNotThrow(() -> ns.dropTable(dropRegisteredTable));
+ DescribeTableRequest describeRegistered = new DescribeTableRequest();
+ describeRegistered.setId(registerIds);
+ Assertions.assertThrows(
+ LanceNamespaceException.class, () ->
ns.describeTable(describeRegistered));
+
+ // Register table with an existing location, then delete the location
before drop
+ List<String> existingIds = List.of(CATALOG_NAME, SCHEMA_NAME,
"existing_then_deleted");
+ String existingLocation = tempDir + "/" + "existing_then_deleted/";
+ new File(existingLocation).mkdirs();
+ Assertions.assertTrue(new File(existingLocation).exists());
+
+ RegisterTableRequest existingRegisterRequest = new RegisterTableRequest();
+ existingRegisterRequest.setId(existingIds);
+ existingRegisterRequest.setLocation(existingLocation);
+ existingRegisterRequest.setMode("create");
+ Assertions.assertDoesNotThrow(() ->
ns.registerTable(existingRegisterRequest));
+
+ FileUtils.deleteQuietly(new File(existingLocation));
+ Assertions.assertFalse(new File(existingLocation).exists());
+
+ DropTableRequest dropExistingTable = new DropTableRequest();
+ dropExistingTable.setId(existingIds);
+ Assertions.assertDoesNotThrow(() -> ns.dropTable(dropExistingTable));
+ DescribeTableRequest describeExisting = new DescribeTableRequest();
+ describeExisting.setId(existingIds);
+ Assertions.assertThrows(
+ LanceNamespaceException.class, () ->
ns.describeTable(describeExisting));
+ }
+
private GravitinoMetalake createMetalake(String metalakeName) {
return client.createMetalake(metalakeName, "metalake for lance rest
service tests", null);
}
