diff --git a/docs/src/hive2.md b/docs/src/hive2.md index a10a280..52222e3 100644 --- a/docs/src/hive2.md +++ b/docs/src/hive2.md @@ -28,7 +28,7 @@ The **namespace identifier** is the database name. ### Table -A **table** is represented as a [Table object](https://github.com/apache/hive/blob/branch-2.3/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Table.java) in HMS with `tableType` set to `EXTERNAL_TABLE`. +A **table** is represented as a [Table object](https://github.com/apache/hive/blob/branch-2.3/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Table.java) in HMS with `tableType` set to `EXTERNAL_TABLE` and the table parameter `EXTERNAL=TRUE`. Both must be set, because HMS silently stores a table declared as `EXTERNAL_TABLE` without the `EXTERNAL=TRUE` parameter as a `MANAGED_TABLE`. The **table identifier** is constructed by joining database and table name with the `$` delimiter (e.g., `database$table`). @@ -38,7 +38,7 @@ The **table location** is stored in the `location` field of the table's `storage ## Lance Table Identification -A table in HMS is identified as a Lance table when it meets the following criteria: the `tableType` is `EXTERNAL_TABLE`, and the `parameters` map contains a key `table_type` with value `lance` (case insensitive). The `location` in `storageDescriptor` may be declared before a Lance dataset exists; storage is checked only for `include_declared=false` listing or `check_declared=true` describe requests. +A table in HMS is identified as a Lance table when the `parameters` map contains a key `table_type` with value `lance` (case insensitive). The `location` in `storageDescriptor` may be declared before a Lance dataset exists; storage is checked only for `include_declared=false` listing or `check_declared=true` describe requests. ## Basic Operations @@ -116,7 +116,7 @@ The implementation: 2. Verify the parent namespace exists 3. Create an HMS Table object with `tableType=EXTERNAL_TABLE` 4. Set the storage descriptor with the specified or default location. When location is not specified, it defaults to `{root}/{database}.db/{table}` -5. Merge request `properties` with required table parameters such as `table_type=lance` and `managed_by=storage` +5. Merge request `properties` with required table parameters such as `table_type=lance`, `managed_by=storage`, and `EXTERNAL=TRUE` 6. Register the table in HMS 7. Return the declared table location, table parameters, and `managed_versioning=false` @@ -175,7 +175,8 @@ The implementation: 1. Parse the table identifier 2. Retrieve the Table object and validate it is a Lance table -3. Drop the table from HMS with `deleteData=true`, which removes both the metadata and the underlying Lance table data +3. Drop the table from HMS with `deleteData=false`, since HMS does not delete data for external tables +4. Delete the underlying Lance dataset at the table location as a best-effort cleanup **Error Handling:** diff --git a/docs/src/hive3.md b/docs/src/hive3.md index 83159d4..885c37d 100644 --- a/docs/src/hive3.md +++ b/docs/src/hive3.md @@ -28,7 +28,7 @@ The **namespace identifier** is constructed by joining namespace levels with the ### Table -A **table** is represented as a [Table object](https://github.com/apache/hive/blob/branch-4.0/standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift#L631) in HMS with `tableType` set to `EXTERNAL_TABLE`. +A **table** is represented as a [Table object](https://github.com/apache/hive/blob/branch-4.0/standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift#L631) in HMS with `tableType` set to `EXTERNAL_TABLE` and the table parameter `EXTERNAL=TRUE`. Both must be set, because HMS silently stores a table declared as `EXTERNAL_TABLE` without the `EXTERNAL=TRUE` parameter as a `MANAGED_TABLE`. The **table identifier** is constructed by joining catalog, database, and table name with the `$` delimiter (e.g., `catalog$database$table`). @@ -38,7 +38,7 @@ The **table location** is stored in the [`location`](https://github.com/apache/h ## Lance Table Identification -A table in HMS is identified as a Lance table when it meets the following criteria: the `tableType` is `EXTERNAL_TABLE`, and the `parameters` map contains a key `table_type` with value `lance` (case insensitive). The `location` in `storageDescriptor` may be declared before a Lance dataset exists; storage is checked only for `include_declared=false` listing or `check_declared=true` describe requests. +A table in HMS is identified as a Lance table when the `parameters` map contains a key `table_type` with value `lance` (case insensitive). The `location` in `storageDescriptor` may be declared before a Lance dataset exists; storage is checked only for `include_declared=false` listing or `check_declared=true` describe requests. ## Basic Operations @@ -123,7 +123,7 @@ The implementation: 2. Verify the parent namespace exists 3. Create an HMS Table object with `tableType=EXTERNAL_TABLE` 4. Set the storage descriptor with the specified or default location. When location is not specified, it defaults to `{root}/{database}.db/{table}` for the default `hive` catalog (hive2-compatible), or `{root}/{catalog}/{database}.db/{table}` for other catalogs -5. Merge request `properties` with required table parameters such as `table_type=lance` and `managed_by=storage` +5. Merge request `properties` with required table parameters such as `table_type=lance`, `managed_by=storage`, and `EXTERNAL=TRUE` 6. Register the table in HMS 7. Return the declared table location, table parameters, and `managed_versioning=false` @@ -182,7 +182,8 @@ The implementation: 1. Parse the table identifier 2. Retrieve the Table object and validate it is a Lance table -3. Drop the table from HMS with `deleteData=true`, which removes both the metadata and the underlying Lance table data +3. Drop the table from HMS with `deleteData=false`, since HMS does not delete data for external tables +4. Delete the underlying Lance dataset at the table location as a best-effort cleanup **Error Handling:** diff --git a/java/lance-namespace-hive2/src/main/java/org/lance/namespace/hive2/Hive2Namespace.java b/java/lance-namespace-hive2/src/main/java/org/lance/namespace/hive2/Hive2Namespace.java index f9933fe..19dea63 100644 --- a/java/lance-namespace-hive2/src/main/java/org/lance/namespace/hive2/Hive2Namespace.java +++ b/java/lance-namespace-hive2/src/main/java/org/lance/namespace/hive2/Hive2Namespace.java @@ -503,6 +503,8 @@ protected void doCreateTable( table.setSd(sd); Map params = Hive2Util.createLanceTableParams(properties); + // HMS silently rewrites the table type to MANAGED_TABLE unless this parameter is set + params.put("EXTERNAL", "TRUE"); table.setParameters(params); clientPool.run( @@ -586,12 +588,18 @@ protected String doDropTable(ObjectIdentifier id, boolean deleteData) { Hive2Util.validateLanceTable(hmsTable.get()); String location = hmsTable.get().getSd().getLocation(); + // HMS does not delete data for external tables, so drop metadata only and delete the + // Lance dataset explicitly when requested clientPool.run( client -> { - client.dropTable(db, tableName, deleteData, true /* ignoreUnknownTable */); + client.dropTable(db, tableName, false /* deleteData */, true /* ignoreUnknownTable */); return null; }); + if (deleteData) { + safeDropDataset(location); + } + return location; } catch (TException | InterruptedException e) { if (e instanceof InterruptedException) { @@ -603,6 +611,14 @@ protected String doDropTable(ObjectIdentifier id, boolean deleteData) { } } + private void safeDropDataset(String location) { + try { + Dataset.drop(location, Collections.emptyMap()); + } catch (Exception e) { + LOG.warn("Failed to delete Lance dataset at {} for dropped table", location, e); + } + } + protected Map doDropNamespace(ObjectIdentifier id, String mode, String behavior) { String db = id.levelAtListPos(0).toLowerCase(); 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..991d947 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 @@ -13,14 +13,19 @@ */ package org.lance.namespace.hive2; +import org.lance.Dataset; +import org.lance.WriteParams; 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.DeregisterTableRequest; import org.lance.namespace.model.DescribeNamespaceRequest; import org.lance.namespace.model.DescribeNamespaceResponse; import org.lance.namespace.model.DescribeTableRequest; import org.lance.namespace.model.DropNamespaceRequest; import org.lance.namespace.model.DropNamespaceResponse; +import org.lance.namespace.model.DropTableRequest; import org.lance.namespace.model.ListTablesRequest; import org.lance.namespace.model.ListTablesResponse; import org.lance.namespace.model.NamespaceExistsRequest; @@ -29,7 +34,11 @@ import com.google.common.collect.Maps; import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.RootAllocator; +import org.apache.arrow.vector.types.pojo.ArrowType; +import org.apache.arrow.vector.types.pojo.Field; +import org.apache.arrow.vector.types.pojo.Schema; 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; @@ -38,12 +47,14 @@ import java.io.File; import java.io.IOException; +import java.util.Collections; import java.util.Map; import static java.nio.file.Files.createTempDirectory; import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute; import static java.nio.file.attribute.PosixFilePermissions.fromString; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -277,6 +288,104 @@ public void testDropNamespaceFailMode() { assertTrue(error.getMessage().contains("Database non_existent_db doesn't exist")); } + @Test + public void testDeclareTableIsExternalInHms() throws Exception { + CreateNamespaceRequest nsRequest = new CreateNamespaceRequest(); + nsRequest.setId(Lists.list("test_db")); + nsRequest.setMode("Create"); + namespace.createNamespace(nsRequest); + + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_db", "ext_table")); + declareRequest.setLocation(tmpDirBase + "/ext_table"); + namespace.declareTable(declareRequest); + + Table hmsTable = metastore.clientPool().run(client -> client.getTable("test_db", "ext_table")); + assertEquals("EXTERNAL_TABLE", hmsTable.getTableType()); + assertEquals("TRUE", hmsTable.getParameters().get("EXTERNAL")); + } + + @Test + public void testDropTableDeletesData() throws Exception { + CreateNamespaceRequest nsRequest = new CreateNamespaceRequest(); + nsRequest.setId(Lists.list("test_db")); + nsRequest.setMode("Create"); + namespace.createNamespace(nsRequest); + + String location = tmpDirBase + "/drop_data_table"; + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_db", "drop_data_table")); + declareRequest.setLocation(location); + namespace.declareTable(declareRequest); + + Schema schema = + new Schema(Collections.singletonList(Field.nullable("id", new ArrowType.Int(32, true)))); + WriteParams writeParams = + new WriteParams.Builder().withMode(WriteParams.WriteMode.CREATE).build(); + Dataset.create(allocator, location, schema, writeParams).close(); + assertTrue(new File(location).exists()); + + DropTableRequest dropRequest = new DropTableRequest(); + dropRequest.setId(Lists.list("test_db", "drop_data_table")); + namespace.dropTable(dropRequest); + + TableExistsRequest existsRequest = new TableExistsRequest(); + existsRequest.setId(Lists.list("test_db", "drop_data_table")); + assertThrows(LanceNamespaceException.class, () -> namespace.tableExists(existsRequest)); + assertFalse(new File(location).exists()); + } + + @Test + public void testDropTableDeclaredOnly() { + CreateNamespaceRequest nsRequest = new CreateNamespaceRequest(); + nsRequest.setId(Lists.list("test_db")); + nsRequest.setMode("Create"); + namespace.createNamespace(nsRequest); + + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_db", "declared_only_table")); + declareRequest.setLocation(tmpDirBase + "/declared_only_table"); + namespace.declareTable(declareRequest); + + // Dropping a declared-only table (no Lance dataset at the location) must still succeed + DropTableRequest dropRequest = new DropTableRequest(); + dropRequest.setId(Lists.list("test_db", "declared_only_table")); + namespace.dropTable(dropRequest); + + TableExistsRequest existsRequest = new TableExistsRequest(); + existsRequest.setId(Lists.list("test_db", "declared_only_table")); + assertThrows(LanceNamespaceException.class, () -> namespace.tableExists(existsRequest)); + } + + @Test + public void testDeregisterTableKeepsData() throws Exception { + CreateNamespaceRequest nsRequest = new CreateNamespaceRequest(); + nsRequest.setId(Lists.list("test_db")); + nsRequest.setMode("Create"); + namespace.createNamespace(nsRequest); + + String location = tmpDirBase + "/deregister_table"; + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_db", "deregister_table")); + declareRequest.setLocation(location); + namespace.declareTable(declareRequest); + + Schema schema = + new Schema(Collections.singletonList(Field.nullable("id", new ArrowType.Int(32, true)))); + WriteParams writeParams = + new WriteParams.Builder().withMode(WriteParams.WriteMode.CREATE).build(); + Dataset.create(allocator, location, schema, writeParams).close(); + + DeregisterTableRequest deregisterRequest = new DeregisterTableRequest(); + deregisterRequest.setId(Lists.list("test_db", "deregister_table")); + namespace.deregisterTable(deregisterRequest); + + TableExistsRequest existsRequest = new TableExistsRequest(); + existsRequest.setId(Lists.list("test_db", "deregister_table")); + assertThrows(LanceNamespaceException.class, () -> namespace.tableExists(existsRequest)); + assertTrue(new File(location).exists()); + } + @Test public void testCloseReleasesClientPool() throws Exception { // A freshly initialized namespace can be used and then closed (idempotently), releasing its diff --git a/java/lance-namespace-hive3/src/main/java/org/lance/namespace/hive3/Hive3Namespace.java b/java/lance-namespace-hive3/src/main/java/org/lance/namespace/hive3/Hive3Namespace.java index 103649e..a22d9bf 100644 --- a/java/lance-namespace-hive3/src/main/java/org/lance/namespace/hive3/Hive3Namespace.java +++ b/java/lance-namespace-hive3/src/main/java/org/lance/namespace/hive3/Hive3Namespace.java @@ -556,6 +556,8 @@ protected void doCreateTable( table.setSd(sd); Map params = Hive3Util.createLanceTableParams(properties); + // HMS silently rewrites the table type to MANAGED_TABLE unless this parameter is set + params.put("EXTERNAL", "TRUE"); table.setParameters(params); clientPool.run( @@ -640,12 +642,19 @@ protected String doDropTable(ObjectIdentifier id, boolean deleteData) { Hive3Util.validateLanceTable(hmsTable.get()); String location = hmsTable.get().getSd().getLocation(); + // HMS does not delete data for external tables, so drop metadata only and delete the + // Lance dataset explicitly when requested clientPool.run( client -> { - client.dropTable(catalog, db, tableName, deleteData, true /* ignoreUnknownTable */); + client.dropTable( + catalog, db, tableName, false /* deleteData */, true /* ignoreUnknownTable */); return null; }); + if (deleteData) { + safeDropDataset(location); + } + return location; } catch (TException | InterruptedException e) { if (e instanceof InterruptedException) { @@ -656,6 +665,14 @@ protected String doDropTable(ObjectIdentifier id, boolean deleteData) { } } + private void safeDropDataset(String location) { + try { + Dataset.drop(location, Collections.emptyMap()); + } catch (Exception e) { + LOG.warn("Failed to delete Lance dataset at {} for dropped table", location, e); + } + } + protected Map doDropNamespace(ObjectIdentifier id, String mode, String behavior) { try { 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..47a36e0 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 @@ -13,15 +13,20 @@ */ package org.lance.namespace.hive3; +import org.lance.Dataset; +import org.lance.WriteParams; import org.lance.namespace.LanceNamespace; 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.DeregisterTableRequest; import org.lance.namespace.model.DescribeNamespaceRequest; import org.lance.namespace.model.DescribeNamespaceResponse; import org.lance.namespace.model.DescribeTableRequest; import org.lance.namespace.model.DropNamespaceRequest; import org.lance.namespace.model.DropNamespaceResponse; +import org.lance.namespace.model.DropTableRequest; import org.lance.namespace.model.ListTablesRequest; import org.lance.namespace.model.ListTablesResponse; import org.lance.namespace.model.NamespaceExistsRequest; @@ -30,7 +35,11 @@ import com.google.common.collect.Maps; import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.RootAllocator; +import org.apache.arrow.vector.types.pojo.ArrowType; +import org.apache.arrow.vector.types.pojo.Field; +import org.apache.arrow.vector.types.pojo.Schema; 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; @@ -39,12 +48,14 @@ import java.io.File; import java.io.IOException; +import java.util.Collections; import java.util.Map; import static java.nio.file.Files.createTempDirectory; import static java.nio.file.attribute.PosixFilePermissions.asFileAttribute; import static java.nio.file.attribute.PosixFilePermissions.fromString; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -388,6 +399,87 @@ public void testDropCatalogRestrictWithDatabases() { assertTrue(error.getMessage().contains("databases")); } + @Test + public void testDeclareTableIsExternalInHms() throws Exception { + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_catalog", "test_db", "ext_table")); + declareRequest.setLocation(tmpDirBase + "/ext_table"); + namespace.declareTable(declareRequest); + + Table hmsTable = + metastore + .clientPool() + .run(client -> client.getTable("test_catalog", "test_db", "ext_table")); + assertEquals("EXTERNAL_TABLE", hmsTable.getTableType()); + assertEquals("TRUE", hmsTable.getParameters().get("EXTERNAL")); + } + + @Test + public void testDropTableDeletesData() throws Exception { + String location = tmpDirBase + "/drop_data_table"; + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_catalog", "test_db", "drop_data_table")); + declareRequest.setLocation(location); + namespace.declareTable(declareRequest); + + Schema schema = + new Schema(Collections.singletonList(Field.nullable("id", new ArrowType.Int(32, true)))); + WriteParams writeParams = + new WriteParams.Builder().withMode(WriteParams.WriteMode.CREATE).build(); + Dataset.create(allocator, location, schema, writeParams).close(); + assertTrue(new File(location).exists()); + + DropTableRequest dropRequest = new DropTableRequest(); + dropRequest.setId(Lists.list("test_catalog", "test_db", "drop_data_table")); + namespace.dropTable(dropRequest); + + TableExistsRequest existsRequest = new TableExistsRequest(); + existsRequest.setId(Lists.list("test_catalog", "test_db", "drop_data_table")); + assertThrows(LanceNamespaceException.class, () -> namespace.tableExists(existsRequest)); + assertFalse(new File(location).exists()); + } + + @Test + public void testDropTableDeclaredOnly() { + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_catalog", "test_db", "declared_only_table")); + declareRequest.setLocation(tmpDirBase + "/declared_only_table"); + namespace.declareTable(declareRequest); + + // Dropping a declared-only table (no Lance dataset at the location) must still succeed + DropTableRequest dropRequest = new DropTableRequest(); + dropRequest.setId(Lists.list("test_catalog", "test_db", "declared_only_table")); + namespace.dropTable(dropRequest); + + TableExistsRequest existsRequest = new TableExistsRequest(); + existsRequest.setId(Lists.list("test_catalog", "test_db", "declared_only_table")); + assertThrows(LanceNamespaceException.class, () -> namespace.tableExists(existsRequest)); + } + + @Test + public void testDeregisterTableKeepsData() throws Exception { + String location = tmpDirBase + "/deregister_table"; + DeclareTableRequest declareRequest = new DeclareTableRequest(); + declareRequest.setId(Lists.list("test_catalog", "test_db", "deregister_table")); + declareRequest.setLocation(location); + namespace.declareTable(declareRequest); + + Schema schema = + new Schema(Collections.singletonList(Field.nullable("id", new ArrowType.Int(32, true)))); + WriteParams writeParams = + new WriteParams.Builder().withMode(WriteParams.WriteMode.CREATE).build(); + Dataset.create(allocator, location, schema, writeParams).close(); + + DeregisterTableRequest deregisterRequest = new DeregisterTableRequest(); + deregisterRequest.setId(Lists.list("test_catalog", "test_db", "deregister_table")); + namespace.deregisterTable(deregisterRequest); + + TableExistsRequest existsRequest = new TableExistsRequest(); + existsRequest.setId(Lists.list("test_catalog", "test_db", "deregister_table")); + assertThrows(LanceNamespaceException.class, () -> namespace.tableExists(existsRequest)); + assertTrue(new File(location).exists()); + } + @Test public void testCloseReleasesClientPool() throws Exception { // A freshly initialized namespace can be used and then closed (idempotently), releasing its