(gravitino) branch branch-1.3 updated: [Cherry-pick to branch-1.3] [#11893] fix(lance): Add external table guard to deregisterTable (#11896) (#11905)

[yuqi4733]

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 2894f13da3 [Cherry-pick to branch-1.3] [#11893] fix(lance): Add 
external table guard to deregisterTable (#11896) (#11905)
2894f13da3 is described below

commit 2894f13da3f8ee1d4faf5a13b54a11e077337599
Author: StormSpirit <jiang...@vip.qq.com>
AuthorDate: Mon Jul 6 11:37:57 2026 +0800

    [Cherry-pick to branch-1.3] [#11893] fix(lance): Add external table guard 
to deregisterTable (#11896) (#11905)
    
    **Cherry-pick Information:**
    - Original commit: 15778b188f10bbd8cd5698eb5788884611b09e0f
    - Target branch: `branch-1.3`
    - Status: ✅ Clean cherry-pick (no conflicts)
    
    Signed-off-by: jiangxt2 <jiang...@vip.qq.com>
---
 .../gravitino/GravitinoLanceTableOperations.java   | 16 +++++++++-
 .../rest/TestGravitinoLanceTableOperations.java    | 34 ++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

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 cad7735118..50ba09df30 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
@@ -281,7 +281,21 @@ public class GravitinoLanceTableOperations implements 
LanceTableOperations {
           "Table not found: " + tableId, CommonUtil.formatCurrentStackTrace(), 
tableId);
     }
     Map<String, String> properties = t.properties();
-    // TODO Support real deregister API.
+
+    // Verify the table is external before deregistering. For non-external 
(managed) tables,
+    // 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);
+    if (!external) {
+      throw new UnsupportedOperationException(
+          "deregisterTable only supports external tables, but table " + 
tableId + " is managed");
+    }
+
+    // External tables: dropTable removes catalog metadata only, preserving 
Lance data.
     boolean result = catalog.asTableCatalog().dropTable(tableIdentifier);
     if (!result) {
       throw new TableNotFoundException(
diff --git 
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestGravitinoLanceTableOperations.java
 
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestGravitinoLanceTableOperations.java
index 10c9b6bf32..84618a0932 100644
--- 
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestGravitinoLanceTableOperations.java
+++ 
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/service/rest/TestGravitinoLanceTableOperations.java
@@ -21,11 +21,17 @@ package org.apache.gravitino.lance.service.rest;
 
 import static 
org.apache.gravitino.lance.common.utils.LanceConstants.LANCE_TABLE_VERSION;
 
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import org.apache.gravitino.Catalog;
+import org.apache.gravitino.NameIdentifier;
+import 
org.apache.gravitino.lance.common.ops.gravitino.GravitinoLanceNamespaceWrapper;
 import 
org.apache.gravitino.lance.common.ops.gravitino.GravitinoLanceTableAlterHandler.AlterColumnsGravitinoLance;
 import 
org.apache.gravitino.lance.common.ops.gravitino.GravitinoLanceTableAlterHandler.DropColumns;
+import 
org.apache.gravitino.lance.common.ops.gravitino.GravitinoLanceTableOperations;
 import org.apache.gravitino.rel.Table;
+import org.apache.gravitino.rel.TableCatalog;
 import org.apache.gravitino.rel.TableChange;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
@@ -92,4 +98,32 @@ class TestGravitinoLanceTableOperations {
     Assertions.assertEquals(
         "Only RENAME alteration is supported currently.", 
exception.getMessage());
   }
+
+  @Test
+  void testDeregisterTableRejectsManagedTable() {
+    // Mock a managed table (no PROPERTY_EXTERNAL=true)
+    Table managedTable = Mockito.mock(Table.class);
+    Mockito.when(managedTable.properties()).thenReturn(new HashMap<>());
+
+    TableCatalog tableCatalog = Mockito.mock(TableCatalog.class);
+    Mockito.when(tableCatalog.loadTable(Mockito.any(NameIdentifier.class)))
+        .thenReturn(managedTable);
+
+    Catalog catalog = Mockito.mock(Catalog.class);
+    Mockito.when(catalog.asTableCatalog()).thenReturn(tableCatalog);
+
+    GravitinoLanceNamespaceWrapper wrapper = 
Mockito.mock(GravitinoLanceNamespaceWrapper.class);
+    
Mockito.when(wrapper.loadAndValidateLakehouseCatalog(Mockito.anyString())).thenReturn(catalog);
+
+    GravitinoLanceTableOperations ops = new 
GravitinoLanceTableOperations(wrapper);
+
+    UnsupportedOperationException exception =
+        Assertions.assertThrows(
+            UnsupportedOperationException.class,
+            () -> ops.deregisterTable("catalog.schema.table", "."));
+    Assertions.assertTrue(exception.getMessage().contains("only supports 
external tables"));
+
+    // Verify dropTable was never called — the guard must reject before 
reaching the catalog layer.
+    Mockito.verify(tableCatalog, Mockito.never()).dropTable(Mockito.any());
+  }
 }