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 01b062cf9a [Cherry-pick to branch-1.3] [#13931] fix(lance): preserve
externally owned registered datasets (#13932) (#13954)
01b062cf9a is described below
commit 01b062cf9a6bbf6b49064d2011dca28f1f0df5c4
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Oct 20 08:19:57 2026 +0800
[Cherry-pick to branch-1.3] [#13931] fix(lance): preserve externally owned
registered datasets (#13932) (#13954)
**Cherry-pick Information:**
- Original commit: f742f1100a5a67c198267965134e98a6d7daa2d9
- Target branch: `branch-1.3`
- Status: ⚠️ **Has conflicts - manual resolution required**
**Do not merge** until conflict markers are resolved and the
`cherry-pick-conflict` label is removed.
Please review and resolve the conflicts before merging.
---------
Co-authored-by: Qi Yu <[email protected]>
---
.../generic/GenericCatalogOperations.java | 2 +
.../lakehouse/lance/LanceTableOperations.java | 11 +-
.../generic/TestGenericCatalogOperations.java | 148 ++++++++++++++++++++
.../lakehouse/lance/TestLanceTableOperations.java | 152 +++++++++++++++++++++
docs/lakehouse-generic-lance-table.md | 9 +-
.../gravitino/GravitinoLanceTableOperations.java | 5 +-
.../lance/common/utils/LancePropertiesUtils.java | 48 +++++++
.../common/utils/TestLancePropertiesUtils.java | 47 +++++++
.../TestGravitinoLanceTableOperations.java | 40 ++++++
9 files changed, 449 insertions(+), 13 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 11497e4d68..5cc15f4e19 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;
@@ -233,6 +234,7 @@ public class GenericCatalogOperations implements
CatalogOperations, SupportsSche
Index[] indexes)
throws NoSuchSchemaException, TableAlreadyExistsException {
Schema schema = loadSchema(NameIdentifier.of(ident.namespace().levels()));
+ properties = LancePropertiesUtils.normalizeTableProperties(properties);
String tableLocation = calculateTableLocation(schema, ident, properties);
String format = properties.getOrDefault(Table.PROPERTY_TABLE_FORMAT, null);
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 86debc4f03..344fcefe44 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
@@ -277,10 +277,7 @@ public class LanceTableOperations extends
ManagedTableOperations {
// Use super.loadTable to avoid triggering an unnecessary schema-refresh
(which may open the
// dataset) for a table that is about to be deleted anyway.
Table table = super.loadTable(ident);
- 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
@@ -313,10 +310,7 @@ public class LanceTableOperations extends
ManagedTableOperations {
try {
// Use super.loadTable to skip schema-refresh overhead when dropping.
Table table = super.loadTable(ident);
- 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
@@ -376,6 +370,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 fc0c0859c1..8d6c25795a 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
@@ -37,14 +37,21 @@ import static
org.apache.gravitino.Configs.STORE_TRANSACTION_MAX_SKEW_TIME;
import static org.apache.gravitino.Configs.VERSION_RETENTION_COUNT;
import static org.mockito.Mockito.when;
+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;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
+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;
@@ -64,6 +71,12 @@ import org.apache.gravitino.meta.AuditInfo;
import org.apache.gravitino.meta.BaseMetalake;
import org.apache.gravitino.meta.CatalogEntity;
import org.apache.gravitino.meta.SchemaVersion;
+import org.apache.gravitino.meta.TableEntity;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.expressions.sorts.SortOrder;
+import org.apache.gravitino.rel.expressions.transforms.Transform;
+import org.apache.gravitino.rel.indexes.Index;
import org.apache.gravitino.storage.IdGenerator;
import org.apache.gravitino.storage.RandomIdGenerator;
import org.apache.gravitino.utils.NameIdentifierUtil;
@@ -71,6 +84,11 @@ import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
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 {
@@ -145,6 +163,136 @@ public class TestGenericCatalogOperations {
ops = new GenericCatalogOperations(store, idGenerator);
}
+ @TempDir private Path lanceDataDir;
+
+ /** Registered tables must be persisted as external whatever the caller
passed. */
+ @ParameterizedTest
+ @CsvSource(
+ value = {"NULL", "false", "true"},
+ nullValues = "NULL")
+ void testRegisterPropertiesAreExternal(String external) {
+ GenericCatalogOperations lanceOps = initializedOps();
+ 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 =
+ lanceOps.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(original, properties);
+ Assertions.assertTrue(lanceOps.dropTable(ident));
+ }
+
+ /** Legacy registrations preserve real data on drop and schema cascade;
purge deletes it. */
+ @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 {
+ GenericCatalogOperations lanceOps = initializedOps();
+ 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(lanceOps.purgeTable(ident));
+ } else if ("cascade".equals(operation)) {
+ Assertions.assertTrue(lanceOps.dropSchema(schemaIdent, true));
+ } else {
+ Assertions.assertTrue(lanceOps.dropTable(ident));
+ }
+
+ Assertions.assertFalse(lanceOps.tableExists(ident));
+ if ("purge".equals(operation)) {
+
Assertions.assertFalse(Files.exists(lanceDataDir.resolve("legacy/_versions")));
+ } else {
+ try (Dataset dataset = Dataset.open().uri(location).build()) {
+ Assertions.assertEquals("id",
dataset.getSchema().getFields().get(0).getName());
+ }
+ }
+ }
+
+ private static GenericCatalogOperations initializedOps() {
+ GenericCatalogOperations initialized = new GenericCatalogOperations(store,
idGenerator);
+ initialized.initialize(ImmutableMap.of(), null /* CatalogInfo, unused */,
new GenericCatalog());
+ return initialized;
+ }
+
+ private NameIdentifier createSchema() {
+ NameIdentifier schemaIdent =
+ NameIdentifierUtil.ofSchema(METALAKE_NAME, CATALOG_NAME,
randomSchemaName());
+ ops.createSchema(
+ schemaIdent,
+ null,
+
StringIdentifier.newPropertiesWithId(StringIdentifier.fromId(idGenerator.nextId()),
null));
+ return schemaIdent;
+ }
+
@AfterAll
public static void tearDown() throws IOException {
ops.close();
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 3545e2beda..530cfe9bd2 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
@@ -36,6 +36,8 @@ import static org.mockito.Mockito.when;
import com.google.common.collect.Maps;
import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.time.Instant;
import java.util.List;
import java.util.Map;
@@ -47,6 +49,7 @@ import org.apache.arrow.vector.types.pojo.Schema;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.StringIdentifier;
import org.apache.gravitino.UserPrincipal;
import org.apache.gravitino.catalog.ManagedSchemaOperations;
import org.apache.gravitino.meta.AuditInfo;
@@ -66,9 +69,11 @@ 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.junit.jupiter.params.provider.ValueSource;
import org.lance.Dataset;
import org.lance.Version;
+import org.lance.WriteParams;
import org.lance.index.IndexOptions;
import org.mockito.InOrder;
import org.mockito.Mockito;
@@ -90,6 +95,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 = writeDataset(location)) {
+ // 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 = writeDataset(location)) {
+ // 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
@@ -1149,4 +1293,12 @@ public class TestLanceTableOperations {
return updated;
});
}
+
+ private static Dataset writeDataset(String location) {
+ return Dataset.write()
+ .schema(new Schema(List.of(Field.nullable("id", new ArrowType.Int(32,
true)))))
+ .uri(location)
+ .mode(WriteParams.WriteMode.CREATE)
+ .execute();
+ }
}
diff --git a/docs/lakehouse-generic-lance-table.md
b/docs/lakehouse-generic-lance-table.md
index 2961527486..0b3bbeb3b9 100644
--- a/docs/lakehouse-generic-lance-table.md
+++ b/docs/lakehouse-generic-lance-table.md
@@ -203,7 +203,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 deab801fee..a658aed474 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
@@ -293,10 +293,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 4d235afcf8..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
@@ -20,11 +20,14 @@
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 {
@@ -33,6 +36,46 @@ public final class LancePropertiesUtils {
// Utility class.
}
+ /**
+ * Returns whether the supplied table format identifies a Lance table.
+ *
+ * @param tableFormat the table format, which may be null
+ * @return true when the format is Lance, ignoring case
+ */
+ public static boolean isLanceTableFormat(String tableFormat) {
+ 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.
*
@@ -95,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 51de5b37e1..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
@@ -18,15 +18,62 @@
*/
package org.apache.gravitino.lance.common.utils;
+import static
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_TABLE_FORMAT;
+
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 {
+ /** Verifies the binary Lance format predicate's case and null handling. */
+ @Test
+ public void testIsLanceTableFormatIsCaseInsensitiveAndNullSafe() {
+
Assertions.assertTrue(LancePropertiesUtils.isLanceTableFormat(LANCE_TABLE_FORMAT));
+ Assertions.assertTrue(LancePropertiesUtils.isLanceTableFormat("LANCE"));
+ Assertions.assertFalse(LancePropertiesUtils.isLanceTableFormat("delta"));
+ 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 a291e80ce1..9af8133485 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
@@ -21,6 +21,7 @@ package org.apache.gravitino.lance.common.ops.gravitino;
import static
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_TABLE_VERSION;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -97,6 +98,37 @@ class TestGravitinoLanceTableOperations {
"Only RENAME alteration is supported currently.",
exception.getMessage());
}
+ @Test
+ void testDeregisterLegacyRegisteredTable() {
+ for (String external : Arrays.asList(null, "false", "true")) {
+ 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(),
+ "external=" + external);
+
+ 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)
@@ -126,4 +158,12 @@ class TestGravitinoLanceTableOperations {
// Verify dropTable was never called — the guard must reject before
reaching the catalog layer.
Mockito.verify(tableCatalog, Mockito.never()).dropTable(Mockito.any());
}
+
+ private static GravitinoLanceTableOperations operations(TableCatalog
tableCatalog) {
+ Catalog catalog = Mockito.mock(Catalog.class);
+ GravitinoLanceNamespaceWrapper wrapper =
Mockito.mock(GravitinoLanceNamespaceWrapper.class);
+
Mockito.when(wrapper.loadAndValidateLakehouseCatalog(Mockito.anyString())).thenReturn(catalog);
+ Mockito.when(wrapper.asTableCatalog(catalog)).thenReturn(tableCatalog);
+ return new GravitinoLanceTableOperations(wrapper);
+ }
}
