From a3b2776ebaa1a7eb07f530dd56f2a5db0c791fcc Mon Sep 17 00:00:00 2001 From: xingsuo-zbz Date: Wed, 1 Jul 2026 16:27:17 +0800 Subject: [PATCH] fix: mark Lance HMS tables as EXTERNAL to prevent data purge on DROP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Add `parameters.EXTERNAL=TRUE` in `Hive2Util`/`Hive3Util` `createLanceTableParams` - HMS's `MetaStoreUtils.isExternalTable()` reads `parameters.EXTERNAL`, not `Table.tableType`. Without this parameter, `DROP TABLE` (Hive CLI, Spark session catalog, HMS's own `drop_table_core`) treats the table as MANAGED and purges the Lance dataset at the storage location — even though `tableType` was already set to `EXTERNAL_TABLE`. - The docs already require Lance HMS tables to be EXTERNAL: https://lance.org/format/namespace/supported-catalogs/hive2/ ## Test plan - New `testDeclareTableMarksTableAsExternal` (hive2 + hive3) — asserts `tableType=EXTERNAL_TABLE` AND `parameters.EXTERNAL=TRUE` are both persisted in HMS after `declareTable`. - New `testDropTablePreservesDataForDeclaredTable` (hive2 + hive3) — declares a table with a data file at its location, drops it via raw HMS `dropTable(deleteData=true)` (the same call path used by Hive CLI / Spark `DROP TABLE`), asserts the data file survives. - Reverting the fix in either `Hive2Util`/`Hive3Util` causes the drop-preserves-data test to fail — the marker file gets purged. - `mvn test -pl lance-namespace-hive2,lance-namespace-hive3 -am`: 31 tests pass, 0 failures. --- .../org/lance/namespace/hive2/Hive2Util.java | 2 + .../namespace/hive2/TestHive2Namespace.java | 55 +++++++++++++++++++ .../org/lance/namespace/hive3/Hive3Util.java | 2 + .../namespace/hive3/TestHive3Namespace.java | 46 ++++++++++++++++ 4 files changed, 105 insertions(+) diff --git a/java/lance-namespace-hive2/src/main/java/org/lance/namespace/hive2/Hive2Util.java b/java/lance-namespace-hive2/src/main/java/org/lance/namespace/hive2/Hive2Util.java index 3d82220..513cda3 100644 --- a/java/lance-namespace-hive2/src/main/java/org/lance/namespace/hive2/Hive2Util.java +++ b/java/lance-namespace-hive2/src/main/java/org/lance/namespace/hive2/Hive2Util.java @@ -124,6 +124,8 @@ public static Map createLanceTableParams(Map pro } params.put("table_type", "lance"); params.put("managed_by", "storage"); + // HMS's MetaStoreUtils.isExternalTable() reads parameters.EXTERNAL, not Table.tableType. + params.put("EXTERNAL", "TRUE"); return params; } } diff --git a/java/lance-namespace-hive2/src/test/java/org/lance/namespace/hive2/TestHive2Namespace.java b/java/lance-namespace-hive2/src/test/java/org/lance/namespace/hive2/TestHive2Namespace.java index d084619..70fe327 100644 --- a/java/lance-namespace-hive2/src/test/java/org/lance/namespace/hive2/TestHive2Namespace.java +++ b/java/lance-namespace-hive2/src/test/java/org/lance/namespace/hive2/TestHive2Namespace.java @@ -16,6 +16,7 @@ import org.lance.namespace.LanceNamespace; import org.lance.namespace.errors.LanceNamespaceException; import org.lance.namespace.model.CreateNamespaceRequest; +import org.lance.namespace.model.DeclareTableRequest; import org.lance.namespace.model.DescribeNamespaceRequest; import org.lance.namespace.model.DescribeNamespaceResponse; import org.lance.namespace.model.DescribeTableRequest; @@ -30,6 +31,7 @@ import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.RootAllocator; import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.api.Table; import org.assertj.core.util.Lists; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; @@ -302,4 +304,57 @@ public void testCloseBeforeInitializeIsSafe() { Hive2Namespace uninitialized = new Hive2Namespace(); uninitialized.close(); } + + @Test + public void testDeclareTableMarksTableAsExternal() throws Exception { + CreateNamespaceRequest nsRequest = new CreateNamespaceRequest(); + nsRequest.setId(Lists.list("ext_db")); + nsRequest.setMode("Create"); + namespace.createNamespace(nsRequest); + + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("ext_db", "ext_tbl")); + declareRequest.setLocation(tmpDirBase + "/ext_db/ext_tbl"); + namespace.declareTable(declareRequest); + + Table hmsTable = metastore.clientPool().run(client -> client.getTable("ext_db", "ext_tbl")); + assertEquals("EXTERNAL_TABLE", hmsTable.getTableType()); + assertEquals("TRUE", hmsTable.getParameters().get("EXTERNAL")); + assertEquals("lance", hmsTable.getParameters().get("table_type")); + assertEquals("storage", hmsTable.getParameters().get("managed_by")); + } + + /** + * Verifies that a table declared via {@link org.lance.namespace.LanceNamespace#declareTable} + * survives a raw HMS {@code dropTable(deleteData=true)} — i.e. the same call path used by Hive + * CLI / Spark {@code DROP TABLE}. Without {@code parameters.EXTERNAL=TRUE}, HMS would treat the + * table as MANAGED and purge the data directory. + */ + @Test + public void testDropTablePreservesDataForDeclaredTable() throws Exception { + CreateNamespaceRequest nsRequest = new CreateNamespaceRequest(); + nsRequest.setId(Lists.list("keep_db")); + nsRequest.setMode("Create"); + namespace.createNamespace(nsRequest); + + File location = new File(tmpDirBase, "keep_db/keep_tbl"); + assertTrue(location.mkdirs()); + File dataFile = new File(location, "data.marker"); + assertTrue(dataFile.createNewFile()); + + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("keep_db", "keep_tbl")); + declareRequest.setLocation(location.getAbsolutePath()); + namespace.declareTable(declareRequest); + + metastore + .clientPool() + .run( + client -> { + client.dropTable("keep_db", "keep_tbl", true, true); + return null; + }); + + assertTrue(dataFile.exists(), "data file was purged: EXTERNAL=TRUE not honored by HMS"); + } } diff --git a/java/lance-namespace-hive3/src/main/java/org/lance/namespace/hive3/Hive3Util.java b/java/lance-namespace-hive3/src/main/java/org/lance/namespace/hive3/Hive3Util.java index 9ffff4c..42681fb 100644 --- a/java/lance-namespace-hive3/src/main/java/org/lance/namespace/hive3/Hive3Util.java +++ b/java/lance-namespace-hive3/src/main/java/org/lance/namespace/hive3/Hive3Util.java @@ -175,6 +175,8 @@ public static Map createLanceTableParams(Map pro } params.put("table_type", "lance"); params.put("managed_by", "storage"); + // HMS's MetaStoreUtils.isExternalTable() reads parameters.EXTERNAL, not Table.tableType. + params.put("EXTERNAL", "TRUE"); return params; } } diff --git a/java/lance-namespace-hive3/src/test/java/org/lance/namespace/hive3/TestHive3Namespace.java b/java/lance-namespace-hive3/src/test/java/org/lance/namespace/hive3/TestHive3Namespace.java index 86bca58..d8f8007 100644 --- a/java/lance-namespace-hive3/src/test/java/org/lance/namespace/hive3/TestHive3Namespace.java +++ b/java/lance-namespace-hive3/src/test/java/org/lance/namespace/hive3/TestHive3Namespace.java @@ -17,6 +17,7 @@ import org.lance.namespace.errors.InvalidInputException; import org.lance.namespace.errors.LanceNamespaceException; import org.lance.namespace.model.CreateNamespaceRequest; +import org.lance.namespace.model.DeclareTableRequest; import org.lance.namespace.model.DescribeNamespaceRequest; import org.lance.namespace.model.DescribeNamespaceResponse; import org.lance.namespace.model.DescribeTableRequest; @@ -31,6 +32,7 @@ import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.RootAllocator; import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.api.Table; import org.assertj.core.util.Lists; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; @@ -416,4 +418,48 @@ public void testCloseBeforeInitializeIsSafe() { Hive3Namespace uninitialized = new Hive3Namespace(); uninitialized.close(); } + + @Test + public void testDeclareTableMarksTableAsExternal() throws Exception { + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_catalog", "test_db", "ext_tbl")); + declareRequest.setLocation(tmpDirBase + "/test_catalog/test_db/ext_tbl"); + namespace.declareTable(declareRequest); + + Table hmsTable = + metastore.clientPool().run(client -> client.getTable("test_catalog", "test_db", "ext_tbl")); + assertEquals("EXTERNAL_TABLE", hmsTable.getTableType()); + assertEquals("TRUE", hmsTable.getParameters().get("EXTERNAL")); + assertEquals("lance", hmsTable.getParameters().get("table_type")); + assertEquals("storage", hmsTable.getParameters().get("managed_by")); + } + + /** + * Verifies that a table declared via {@link org.lance.namespace.LanceNamespace#declareTable} + * survives a raw HMS {@code dropTable(deleteData=true)} — i.e. the same call path used by Hive + * CLI / Spark {@code DROP TABLE}. Without {@code parameters.EXTERNAL=TRUE}, HMS would treat the + * table as MANAGED and purge the data directory. + */ + @Test + public void testDropTablePreservesDataForDeclaredTable() throws Exception { + File location = new File(tmpDirBase, "test_catalog/test_db/keep_tbl"); + assertTrue(location.mkdirs()); + File dataFile = new File(location, "data.marker"); + assertTrue(dataFile.createNewFile()); + + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_catalog", "test_db", "keep_tbl")); + declareRequest.setLocation(location.getAbsolutePath()); + namespace.declareTable(declareRequest); + + metastore + .clientPool() + .run( + client -> { + client.dropTable("test_catalog", "test_db", "keep_tbl", true, true); + return null; + }); + + assertTrue(dataFile.exists(), "data file was purged: EXTERNAL=TRUE not honored by HMS"); + } }