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 f742f1100a [#13931] fix(lance): preserve externally owned registered
datasets (#13932)
f742f1100a is described below
commit f742f1100a5a67c198267965134e98a6d7daa2d9
Author: Qi Yu <[email protected]>
AuthorDate: Mon Oct 19 17:19:42 2026 +0800
[#13931] fix(lance): preserve externally owned registered datasets (#13932)
### What changes were proposed in this pull request?
Persist `external=true` for registered Lance tables, including when the
caller omits the flag or sets it to false. Normalize the properties
before consulting a table location provider as well as in the Lance
registration path.
Use a shared ownership predicate for Lance drop/purge, generic location
reclamation and Lance REST deregistration, so older registrations with
missing/false stored flags also preserve data during ordinary removal.
Document the distinction between drop/deregister and explicit purge.
### Why are the changes needed?
A registration only stores catalog metadata, but the default managed
classification currently allows ordinary drop to delete the existing
dataset. Fixing new registrations alone would leave existing records and
location-provider callbacks unsafe.
Fix: #13931
### Does this PR introduce _any_ user-facing change?
Registered Lance tables are external. Ordinary drop, registration
overwrite of a historical registration, schema cascade and
deregistration preserve the registered dataset. Explicit purge and Lance
REST drop remain destructive. Historical stored properties are not
automatically migrated, and non-Lance ownership rules are unchanged.
### How was this patch tested?
- Ran Spotless for all three affected modules.
- Ran `./gradlew :lance:lance-common:test
:catalogs:catalog-lakehouse-generic:test :lance:lance-rest-server:test
-PskipITs -PskipDockerTests=true`: 370 non-Docker tests passed,
including 32 added cases.
- Tests create real local Lance datasets and verify data preservation
through drop, legacy registration overwrite and schema cascade, as well
as explicit purge deletion, provider reclamation and REST deregistration
routing.
- Reverted ownership normalization/protection temporarily: 14 of 23
selected regression cases failed as expected. Restored the fix and reran
all three module suites successfully.
- Docker and deployed client/server E2E tests were not run.
---
.../generic/GenericCatalogOperations.java | 5 +-
.../lakehouse/lance/LanceTableOperations.java | 11 +-
.../generic/TestGenericCatalogOperations.java | 136 ++++++++++++++++++++
.../lakehouse/lance/TestLanceTableOperations.java | 139 +++++++++++++++++++++
docs/lakehouse-generic-lance-table.md | 9 +-
.../gravitino/GravitinoLanceTableOperations.java | 5 +-
.../lance/common/utils/LancePropertiesUtils.java | 37 ++++++
.../common/utils/TestLancePropertiesUtils.java | 36 ++++++
.../TestGravitinoLanceTableOperations.java | 33 +++++
9 files changed, 397 insertions(+), 14 deletions(-)
diff --git
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalogOperations.java
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalogOperations.java
index 54999ca482..fa6626aca4 100644
---
a/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalogOperations.java
+++
b/catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalogOperations.java
@@ -56,6 +56,7 @@ import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.exceptions.NonEmptySchemaException;
import org.apache.gravitino.exceptions.SchemaAlreadyExistsException;
import org.apache.gravitino.exceptions.TableAlreadyExistsException;
+import org.apache.gravitino.lance.common.utils.LancePropertiesUtils;
import org.apache.gravitino.meta.TableEntity;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.Table;
@@ -287,6 +288,7 @@ public class GenericCatalogOperations implements
CatalogOperations, SupportsSche
// table properties, and the built-in provider returns it verbatim as its
first branch, so a
// catalog on the built-in provider is unaffected. What an allocating
provider does with it is
// the provider's decision: honour it, rewrite it, or reject the creation.
+ properties = LancePropertiesUtils.normalizeTableProperties(properties);
String tableLocation = provisionTableLocation(ident, properties, schema);
Map<String, String> newProperties = Maps.newHashMap(properties);
@@ -438,7 +440,8 @@ public class GenericCatalogOperations implements
CatalogOperations, SupportsSche
// Read the same way the table formats and the property metadata read it:
a catalog that
// disagreed with the format about a value like `external=yes` would skip
reclaiming a location
// the format had just deleted the data under.
- boolean external =
Boolean.parseBoolean(context.tableProperties().get(Table.PROPERTY_EXTERNAL));
+ // Use the same effective ownership as the format, including legacy Lance
registrations.
+ boolean external =
LancePropertiesUtils.isExternalTable(context.tableProperties());
if (dropped && (purge || !external)) {
TableLocationProvider provider = tableLocationProvider;
try {
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 48d7d955d9..e93913af95 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
@@ -290,10 +290,7 @@ public class LanceTableOperations extends
ManagedTableOperations {
// dataset) for a table that is about to be deleted anyway.
Table table = super.loadTable(ident);
validateLanceTable(ident, table);
- boolean external =
- Optional.ofNullable(table.properties().get(Table.PROPERTY_EXTERNAL))
- .map(Boolean::parseBoolean)
- .orElse(false);
+ boolean external =
LancePropertiesUtils.isExternalTable(table.properties());
// 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
@@ -333,10 +330,7 @@ public class LanceTableOperations extends
ManagedTableOperations {
// Use super.loadTable to skip schema-refresh overhead when dropping.
Table table = super.loadTable(ident);
validateLanceTable(ident, table);
- boolean external =
- Optional.ofNullable(table.properties().get(Table.PROPERTY_EXTERNAL))
- .map(Boolean::parseBoolean)
- .orElse(false);
+ boolean external =
LancePropertiesUtils.isExternalTable(table.properties());
// 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
@@ -409,6 +403,7 @@ public class LanceTableOperations extends
ManagedTableOperations {
throws NoSuchSchemaException, TableAlreadyExistsException {
if (register) {
+ properties = LancePropertiesUtils.normalizeTableProperties(properties);
// Currently, register operation does not read the schema from the
underlying Lance dataset.
// So we can't get the version of the dataset here.
return super.createTable(
diff --git
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/generic/TestGenericCatalogOperations.java
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/generic/TestGenericCatalogOperations.java
index 5e86bd2ffa..7daaed9a8d 100644
---
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/generic/TestGenericCatalogOperations.java
+++
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/generic/TestGenericCatalogOperations.java
@@ -50,6 +50,8 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.time.Instant;
import java.util.Arrays;
import java.util.List;
@@ -58,6 +60,8 @@ import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.Field;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Catalog;
@@ -92,6 +96,11 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.lance.Dataset;
+import org.lance.WriteParams;
import org.mockito.Mockito;
public class TestGenericCatalogOperations {
@@ -192,6 +201,133 @@ public class TestGenericCatalogOperations {
ImmutableMap.of(), null /* CatalogInfo, unused */, new
GenericCatalog());
}
+ @TempDir private Path lanceDataDir;
+
+ /** The provider must see effective ownership before allocating a registered
table location. */
+ @ParameterizedTest
+ @CsvSource(
+ value = {"NULL", "false", "true"},
+ nullValues = "NULL")
+ void testRegisterPropertiesAreExternalBeforeProvisioning(String external) {
+ NameIdentifier schemaIdent = createSchema();
+ NameIdentifier ident =
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaIdent.name(),
"registered");
+ Map<String, String> properties =
+ Maps.newHashMap(
+ Map.of(
+ Table.PROPERTY_TABLE_FORMAT,
+ "lance",
+ "lance.register",
+ "true",
+ Table.PROPERTY_LOCATION,
+ lanceDataDir.resolve("existing").toString()));
+ properties.putAll(
+
StringIdentifier.newPropertiesWithId(StringIdentifier.fromId(idGenerator.nextId()),
null));
+ if (external != null) {
+ properties.put(Table.PROPERTY_EXTERNAL, external);
+ }
+ Map<String, String> original = Map.copyOf(properties);
+
+ Table created =
+ opsWithFakeProvider.createTable(
+ ident,
+ new Column[0],
+ null,
+ properties,
+ new Transform[0],
+ null,
+ new SortOrder[0],
+ new Index[0]);
+
+ Assertions.assertEquals("true",
created.properties().get(Table.PROPERTY_EXTERNAL));
+ Assertions.assertEquals(
+ "true",
+ FakeTableLocationProvider.provisioned()
+ .get(0)
+ .tableProperties()
+ .get(Table.PROPERTY_EXTERNAL));
+ Assertions.assertEquals(original, properties);
+ Assertions.assertTrue(opsWithFakeProvider.dropTable(ident));
+ Assertions.assertTrue(FakeTableLocationProvider.unprovisioned().isEmpty());
+ }
+
+ /** Legacy registrations preserve real data and avoid provider reclamation
on drop/cascade. */
+ @ParameterizedTest
+ @CsvSource(
+ value = {
+ "NULL,drop",
+ "false,drop",
+ "NULL,purge",
+ "false,purge",
+ "NULL,cascade",
+ "false,cascade"
+ },
+ nullValues = "NULL")
+ void testLegacyRegisteredLanceOwnership(String external, String operation)
throws Exception {
+ NameIdentifier schemaIdent = createSchema();
+ String location = lanceDataDir.resolve("legacy").toString();
+ // Arrow Schema and Gravitino Schema share a simple name in this test
class.
+ try (Dataset ignored =
+ Dataset.write()
+ .schema(
+ new org.apache.arrow.vector.types.pojo.Schema(
+ List.of(Field.nullable("id", new ArrowType.Int(32,
true)))))
+ .uri(location)
+ .mode(WriteParams.WriteMode.CREATE)
+ .execute()) {
+ // The dataset predates its catalog registration.
+ }
+ NameIdentifier ident =
+ NameIdentifier.of(METALAKE_NAME, CATALOG_NAME, schemaIdent.name(),
"legacy");
+ Map<String, String> properties =
+ Maps.newHashMap(
+ Map.of(
+ Table.PROPERTY_TABLE_FORMAT,
+ "lance",
+ "lance.register",
+ "true",
+ Table.PROPERTY_LOCATION,
+ location));
+ if (external != null) {
+ properties.put(Table.PROPERTY_EXTERNAL, external);
+ }
+ TableEntity entity =
+ TableEntity.builder()
+ .withId(idGenerator.nextId())
+ .withName(ident.name())
+ .withNamespace(ident.namespace())
+ .withProperties(properties)
+ .withAuditInfo(
+
AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build())
+ .build();
+ store.put(entity, false);
+
+ if ("purge".equals(operation)) {
+ Assertions.assertTrue(opsWithFakeProvider.purgeTable(ident));
+ } else if ("cascade".equals(operation)) {
+ Assertions.assertTrue(opsWithFakeProvider.dropSchema(schemaIdent, true));
+ } else {
+ Assertions.assertTrue(opsWithFakeProvider.dropTable(ident));
+ }
+
+ Assertions.assertFalse(opsWithFakeProvider.tableExists(ident));
+ if ("purge".equals(operation)) {
+
Assertions.assertFalse(Files.exists(lanceDataDir.resolve("legacy/_versions")));
+ Assertions.assertEquals(1,
FakeTableLocationProvider.unprovisioned().size());
+ Assertions.assertEquals(
+ location,
+ FakeTableLocationProvider.unprovisioned()
+ .get(0)
+ .tableProperties()
+ .get(Table.PROPERTY_LOCATION));
+ } else {
+
Assertions.assertTrue(FakeTableLocationProvider.unprovisioned().isEmpty());
+ try (Dataset dataset = Dataset.open().uri(location).build()) {
+ Assertions.assertEquals("id",
dataset.getSchema().getFields().get(0).getName());
+ }
+ }
+ }
+
@BeforeEach
public void resetProvider() {
FakeTableLocationProvider.reset();
diff --git
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java
index bf48d6464f..3afa32e3c2 100644
---
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java
+++
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java
@@ -102,6 +102,145 @@ public class TestLanceTableOperations {
lanceTableOps = spy(new LanceTableOperations(store, schemaOps,
idGenerator));
}
+ /** Registered tables persist external ownership and retain the existing
dataset on drop. */
+ @ParameterizedTest
+ @CsvSource(
+ value = {"NULL", "false", "true"},
+ nullValues = "NULL")
+ void testRegisterPersistsExternalAndDropPreservesDataset(String external)
throws Exception {
+ NameIdentifier ident = NameIdentifier.of("schema", "registered");
+ String location = tempDir.resolve("registered").toString();
+ Column[] columns = {Column.of("id", Types.IntegerType.get())};
+ when(idGenerator.nextId()).thenReturn(10L);
+ try (Dataset ignored = lanceTableOps.createDataset(location, columns,
Map.of())) {
+ // Seed a dataset owned by the caller before registering it.
+ }
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put(Table.PROPERTY_TABLE_FORMAT, "lance");
+ properties.put(Table.PROPERTY_LOCATION, location);
+ properties.put(LANCE_TABLE_REGISTER, "true");
+
properties.putAll(StringIdentifier.newPropertiesWithId(StringIdentifier.fromId(1L),
null));
+ if (external != null) {
+ properties.put(Table.PROPERTY_EXTERNAL, external);
+ }
+ Map<String, String> original = Map.copyOf(properties);
+ AtomicReference<TableEntity> stored = new AtomicReference<>();
+ Mockito.doAnswer(
+ invocation -> {
+ stored.set(invocation.getArgument(0));
+ return null;
+ })
+ .when(store)
+ .put(any(TableEntity.class), eq(false));
+ when(store.get(eq(ident), eq(Entity.EntityType.TABLE),
eq(TableEntity.class)))
+ .thenAnswer(invocation -> stored.get());
+ when(store.delete(ident, Entity.EntityType.TABLE)).thenReturn(true);
+
+ Table created =
+ PrincipalUtils.doAs(
+ new UserPrincipal("tester"),
+ () ->
+ lanceTableOps.createTable(
+ ident,
+ columns,
+ null,
+ properties,
+ new Transform[0],
+ null,
+ new SortOrder[0],
+ new Index[0]));
+
+ Assertions.assertEquals("true",
created.properties().get(Table.PROPERTY_EXTERNAL));
+ Assertions.assertEquals("true",
stored.get().properties().get(Table.PROPERTY_EXTERNAL));
+ Assertions.assertEquals(
+ "true",
lanceTableOps.loadTable(ident).properties().get(Table.PROPERTY_EXTERNAL));
+ Assertions.assertEquals(original, properties);
+ Assertions.assertTrue(lanceTableOps.dropTable(ident));
+ verify(store).delete(ident, Entity.EntityType.TABLE);
+ try (Dataset dataset = Dataset.open().uri(location).build()) {
+ Assertions.assertEquals("id",
dataset.getSchema().getFields().get(0).getName());
+ }
+ }
+
+ /** Legacy registration flags also protect drop and overwrite, while purge
remains destructive. */
+ @ParameterizedTest
+ @CsvSource(
+ value = {
+ "NULL,drop",
+ "false,drop",
+ "true,drop",
+ "NULL,purge",
+ "false,purge",
+ "true,purge",
+ "NULL,overwrite",
+ "false,overwrite"
+ },
+ nullValues = "NULL")
+ void testLegacyRegistrationRemoval(String external, String operation) throws
Exception {
+ NameIdentifier ident = NameIdentifier.of("schema", "legacy");
+ String location = tempDir.resolve("legacy").toString();
+ Column[] columns = {Column.of("id", Types.IntegerType.get())};
+ when(idGenerator.nextId()).thenReturn(10L);
+ try (Dataset ignored = lanceTableOps.createDataset(location, columns,
Map.of())) {
+ // A historical registration must never transfer ownership to Gravitino
implicitly.
+ }
+ Map<String, String> properties =
+ Maps.newHashMap(
+ Map.of(
+ Table.PROPERTY_TABLE_FORMAT,
+ "lance",
+ Table.PROPERTY_LOCATION,
+ location,
+ LANCE_TABLE_REGISTER,
+ "true"));
+ if (external != null) {
+ properties.put(Table.PROPERTY_EXTERNAL, external);
+ }
+ AtomicReference<TableEntity> stored =
+ new AtomicReference<>(tableEntity(ident, List.of(), properties));
+ when(store.get(eq(ident), eq(Entity.EntityType.TABLE),
eq(TableEntity.class)))
+ .thenAnswer(invocation -> stored.get());
+ when(store.delete(ident, Entity.EntityType.TABLE)).thenReturn(true);
+ if ("overwrite".equals(operation)) {
+ Map<String, String> replacement = Maps.newHashMap(properties);
+ replacement.put(LANCE_CREATION_MODE, "OVERWRITE");
+
replacement.putAll(StringIdentifier.newPropertiesWithId(StringIdentifier.fromId(2L),
null));
+ Mockito.doAnswer(
+ invocation -> {
+ stored.set(invocation.getArgument(0));
+ return null;
+ })
+ .when(store)
+ .put(any(TableEntity.class), eq(false));
+ Table replaced =
+ PrincipalUtils.doAs(
+ new UserPrincipal("tester"),
+ () ->
+ lanceTableOps.createTable(
+ ident,
+ columns,
+ null,
+ replacement,
+ new Transform[0],
+ null,
+ new SortOrder[0],
+ new Index[0]));
+ Assertions.assertEquals("true",
replaced.properties().get(Table.PROPERTY_EXTERNAL));
+ } else if ("purge".equals(operation)) {
+ Assertions.assertTrue(lanceTableOps.purgeTable(ident));
+ } else {
+ Assertions.assertTrue(lanceTableOps.dropTable(ident));
+ }
+ verify(store).delete(ident, Entity.EntityType.TABLE);
+ if ("purge".equals(operation)) {
+ Assertions.assertFalse(Files.exists(Paths.get(location, "_versions")));
+ } else {
+ try (Dataset dataset = Dataset.open().uri(location).build()) {
+ Assertions.assertEquals("id",
dataset.getSchema().getFields().get(0).getName());
+ }
+ }
+ }
+
@Test
public void testCreationModeEnum() {
// Test that CreationMode enum has expected values
diff --git a/docs/lakehouse-generic-lance-table.md
b/docs/lakehouse-generic-lance-table.md
index 28bc92e88e..92afc8acb7 100644
--- a/docs/lakehouse-generic-lance-table.md
+++ b/docs/lakehouse-generic-lance-table.md
@@ -297,7 +297,14 @@ tableCatalog.createTable(
#### Register External Tables
-Register existing Lance tables without moving or copying data:
+Register existing Lance tables without moving or copying data. A table with
`lance.register=true`
+is always external, including when the request omits `external` or sets it to
`false`. Dropping it
+through the Gravitino table API removes only the metadata. Explicit
`purgeTable` and Lance REST
+`drop` still delete both the metadata and the dataset; use Lance REST
`deregister` to preserve data.
+
+Existing registrations with a missing or false stored `external` flag receive
the same protection
+when dropped or deregistered. Their stored properties are not automatically
migrated.
+
<Tabs groupId='language' queryString>
<TabItem value="shell" label="Shell">
diff --git
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java
index 300e678246..b72455972d 100644
---
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java
+++
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java
@@ -289,10 +289,7 @@ public class GravitinoLanceTableOperations implements
LanceTableOperations {
// dropTable would delete the underlying Lance data, violating the
deregister contract
// ("It will not delete the underlying lance data"). The TableCatalog
interface does not
// expose a metadata-only removal path, so we fail fast rather than risk
silent data loss.
- boolean external =
- Optional.ofNullable(properties.get(Table.PROPERTY_EXTERNAL))
- .map(Boolean::parseBoolean)
- .orElse(false);
+ boolean external = LancePropertiesUtils.isExternalTable(properties);
if (!external) {
throw new UnsupportedOperationException(
"deregisterTable only supports external tables, but table " +
tableId + " is managed");
diff --git
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/LancePropertiesUtils.java
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/LancePropertiesUtils.java
index aba4087aa2..29cb5db621 100644
---
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/LancePropertiesUtils.java
+++
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/LancePropertiesUtils.java
@@ -21,11 +21,13 @@ package org.apache.gravitino.lance.common.utils;
import static
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_STORAGE_OPTIONS_PREFIX;
import static
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_TABLE_FORMAT;
+import static
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_TABLE_REGISTER;
import com.google.common.base.Preconditions;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.stream.Collectors;
+import org.apache.gravitino.rel.Table;
/** Utility methods for Lance storage properties. */
public final class LancePropertiesUtils {
@@ -44,6 +46,36 @@ public final class LancePropertiesUtils {
return LANCE_TABLE_FORMAT.equalsIgnoreCase(tableFormat);
}
+ /**
+ * Returns whether the table's data is externally owned.
+ *
+ * <p>Registered Lance tables are external even when an older registration
omitted or incorrectly
+ * stored the external flag. Other formats retain their explicit external
flag semantics.
+ *
+ * @param tableProperties the stored table properties
+ * @return true for an explicitly external table or a registered Lance table
+ */
+ public static boolean isExternalTable(Map<String, String> tableProperties) {
+ return Boolean.parseBoolean(tableProperties.get(Table.PROPERTY_EXTERNAL))
+ || isRegisteredLanceTable(tableProperties);
+ }
+
+ /**
+ * Makes registered Lance table properties explicitly external without
changing the input map.
+ *
+ * @param tableProperties the table properties
+ * @return a copy with external=true for a registered Lance table, or the
input when unchanged
+ */
+ public static Map<String, String> normalizeTableProperties(Map<String,
String> tableProperties) {
+ if (!isRegisteredLanceTable(tableProperties)
+ || "true".equals(tableProperties.get(Table.PROPERTY_EXTERNAL))) {
+ return tableProperties;
+ }
+ Map<String, String> normalized = new LinkedHashMap<>(tableProperties);
+ normalized.put(Table.PROPERTY_EXTERNAL, "true");
+ return normalized;
+ }
+
/**
* Extracts Lance storage options from a property map.
*
@@ -106,4 +138,9 @@ public final class LancePropertiesUtils {
Collectors.toMap(
entry -> LANCE_STORAGE_OPTIONS_PREFIX + entry.getKey(),
Map.Entry::getValue));
}
+
+ private static boolean isRegisteredLanceTable(Map<String, String>
tableProperties) {
+ return isLanceTableFormat(tableProperties.get(Table.PROPERTY_TABLE_FORMAT))
+ && Boolean.parseBoolean(tableProperties.get(LANCE_TABLE_REGISTER));
+ }
}
diff --git
a/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestLancePropertiesUtils.java
b/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestLancePropertiesUtils.java
index 3a862c7c06..128f352b61 100644
---
a/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestLancePropertiesUtils.java
+++
b/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/utils/TestLancePropertiesUtils.java
@@ -22,10 +22,13 @@ import static
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_TABLE
import com.google.common.collect.ImmutableMap;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
public class TestLancePropertiesUtils {
@@ -38,6 +41,39 @@ public class TestLancePropertiesUtils {
Assertions.assertFalse(LancePropertiesUtils.isLanceTableFormat(null));
}
+ /** Checks ownership without extending Lance registration semantics to other
formats. */
+ @ParameterizedTest
+ @CsvSource(
+ value = {
+ "lance,true,NULL,true", "LANCE,TRUE,false,true",
"lance,true,true,true",
+ "lance,false,false,false", "lance,NULL,NULL,false",
"lance,false,true,true",
+ "delta,true,false,false", "delta,true,true,true",
"NULL,true,false,false"
+ },
+ nullValues = "NULL")
+ void testEffectiveExternalOwnership(
+ String format, String register, String external, boolean expected) {
+ Map<String, String> properties = new HashMap<>();
+ if (format != null) {
+ properties.put("format", format);
+ }
+ if (register != null) {
+ properties.put("lance.register", register);
+ }
+ if (external != null) {
+ properties.put("external", external);
+ }
+ Map<String, String> original = Map.copyOf(properties);
+
+ Assertions.assertEquals(expected,
LancePropertiesUtils.isExternalTable(properties));
+ Map<String, String> normalized =
LancePropertiesUtils.normalizeTableProperties(properties);
+ if (LancePropertiesUtils.isLanceTableFormat(format) &&
Boolean.parseBoolean(register)) {
+ Assertions.assertEquals("true", normalized.get("external"));
+ } else {
+ Assertions.assertSame(properties, normalized);
+ }
+ Assertions.assertEquals(original, properties);
+ }
+
@Test
public void testGetLanceStorageOptions() {
Map<String, String> properties =
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceTableOperations.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceTableOperations.java
index 079a3a1cd5..9c9f1dfe14 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceTableOperations.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestGravitinoLanceTableOperations.java
@@ -33,6 +33,8 @@ import org.apache.gravitino.rel.TableCatalog;
import org.apache.gravitino.rel.TableChange;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
import org.lance.namespace.errors.InvalidInputException;
import org.lance.namespace.model.AlterColumnsEntry;
import org.lance.namespace.model.AlterTableAlterColumnsRequest;
@@ -98,6 +100,37 @@ class TestGravitinoLanceTableOperations {
"Only RENAME alteration is supported currently.",
exception.getMessage());
}
+ @ParameterizedTest
+ @CsvSource(
+ value = {"NULL", "false", "true"},
+ nullValues = "NULL")
+ void testDeregisterLegacyRegisteredTable(String external) {
+ Table table = Mockito.mock(Table.class);
+ Map<String, String> properties =
+ new HashMap<>(
+ Map.of(
+ Table.PROPERTY_TABLE_FORMAT,
+ "lance",
+ "lance.register",
+ "true",
+ "location",
+ "/existing"));
+ if (external != null) {
+ properties.put(Table.PROPERTY_EXTERNAL, external);
+ }
+ Mockito.when(table.properties()).thenReturn(properties);
+ TableCatalog tableCatalog = Mockito.mock(TableCatalog.class);
+
Mockito.when(tableCatalog.loadTable(Mockito.any(NameIdentifier.class))).thenReturn(table);
+
Mockito.when(tableCatalog.dropTable(Mockito.any(NameIdentifier.class))).thenReturn(true);
+
+ Assertions.assertEquals(
+ "/existing",
+ operations(tableCatalog).deregisterTable("catalog.schema.table",
".").getLocation());
+
+ Mockito.verify(tableCatalog).dropTable(NameIdentifier.of("schema",
"table"));
+ Mockito.verify(tableCatalog, Mockito.never()).purgeTable(Mockito.any());
+ }
+
@Test
void testDeregisterTableRejectsManagedTable() {
// Mock a managed table (no PROPERTY_EXTERNAL=true)
