From 768d7c6743ee9f801be780523f6083651168cca6 Mon Sep 17 00:00:00 2001 From: nk1506 Date: Wed, 25 Oct 2023 16:25:47 +0530 Subject: [PATCH] Hive: Refactor TestHiveCatalog tests to use the core CatalogTests --- build.gradle | 1 + .../apache/iceberg/catalog/CatalogTests.java | 8 + .../org/apache/iceberg/hive/HiveCatalog.java | 16 +- .../iceberg/hive/HiveTableOperations.java | 2 +- .../hive/HiveCreateReplaceTableTest.java | 9 +- .../iceberg/hive/HiveMetastoreExtension.java | 9 +- .../iceberg/hive/HiveTableBaseTest.java | 10 +- .../iceberg/hive/TestCachedClientPool.java | 3 +- .../apache/iceberg/hive/TestHiveCatalog.java | 307 +++--------------- .../iceberg/hive/TestHiveCommitLocks.java | 8 +- 10 files changed, 87 insertions(+), 286 deletions(-) diff --git a/build.gradle b/build.gradle index 3f76cbea02bf..ab0924963906 100644 --- a/build.gradle +++ b/build.gradle @@ -723,6 +723,7 @@ project(':iceberg-hive-metastore') { } testImplementation project(path: ':iceberg-api', configuration: 'testArtifacts') + testImplementation project(path: ':iceberg-core', configuration: 'testArtifacts') testImplementation libs.awaitility } } diff --git a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java index e01deebf5bc5..00f3739e2b9e 100644 --- a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java +++ b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java @@ -164,6 +164,10 @@ protected boolean supportsNamesWithSlashes() { return true; } + protected boolean supportsNamesWithDot() { + return true; + } + @Test public void testCreateNamespace() { C catalog = catalog(); @@ -470,6 +474,8 @@ public void testNamespaceWithSlash() { @Test public void testNamespaceWithDot() { + Assumptions.assumeTrue(supportsNamesWithDot()); + C catalog = catalog(); Namespace withDot = Namespace.of("new.db"); @@ -547,6 +553,8 @@ public void testTableNameWithSlash() { @Test public void testTableNameWithDot() { + Assumptions.assumeTrue(supportsNamesWithDot()); + C catalog = catalog(); TableIdentifier ident = TableIdentifier.of("ns", "ta.ble"); diff --git a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java index 46a8d33c48ac..bfd04e2d05fe 100644 --- a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java +++ b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java @@ -261,6 +261,17 @@ public void renameTable(TableIdentifier from, TableIdentifier originalTo) { } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new RuntimeException("Interrupted in call to rename", e); + } catch (RuntimeException e) { + // in case of table already exists, + // Hive rename operation throws exception as + // java.lang.RuntimeException:InvalidOperationException(message:new table <> already exists) + if (e.getCause() instanceof InvalidOperationException + && e.getCause().getMessage() != null + && e.getCause().getMessage().contains(String.format("new table %s already exists", to))) { + throw new org.apache.iceberg.exceptions.AlreadyExistsException( + "Table already exists: %s", to); + } + throw new RuntimeException("Failed to rename " + from + " to " + to, e); } } @@ -288,7 +299,7 @@ public void createNamespace(Namespace namespace, Map meta) { } catch (AlreadyExistsException e) { throw new org.apache.iceberg.exceptions.AlreadyExistsException( - e, "Namespace '%s' already exists!", namespace); + e, "Namespace already exists: %s", namespace); } catch (TException e) { throw new RuntimeException( @@ -500,6 +511,9 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) { return String.format("%s/%s", databaseData.getLocationUri(), tableIdentifier.name()); } + } catch (NoSuchObjectException e) { + throw new NoSuchNamespaceException( + e, "Namespace does not exist: %s", tableIdentifier.namespace().levels()[0]); } catch (TException e) { throw new RuntimeException( String.format("Metastore operation failed for %s", tableIdentifier), e); diff --git a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java index 5e2d93a01d13..a3750b9f3101 100644 --- a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java +++ b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java @@ -209,7 +209,7 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { String baseMetadataLocation = base != null ? base.metadataFileLocation() : null; if (!Objects.equals(baseMetadataLocation, metadataLocation)) { throw new CommitFailedException( - "Base metadata location '%s' is not same as the current table metadata location '%s' for %s.%s", + "Cannot commit: Base metadata location '%s' is not same as the current table metadata location '%s' for %s.%s", baseMetadataLocation, metadataLocation, database, tableName); } diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveCreateReplaceTableTest.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveCreateReplaceTableTest.java index af5f7ce7335a..f2db5ed4fb11 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveCreateReplaceTableTest.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveCreateReplaceTableTest.java @@ -26,6 +26,7 @@ import java.nio.file.Path; import java.util.Collections; import java.util.concurrent.TimeUnit; +import org.apache.hadoop.hive.metastore.api.Database; import org.apache.iceberg.AppendFiles; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.CatalogUtil; @@ -43,6 +44,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.types.Types; +import org.apache.thrift.TException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -68,12 +70,12 @@ public class HiveCreateReplaceTableTest { @RegisterExtension private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = - new HiveMetastoreExtension(DB_NAME, Collections.emptyMap()); + new HiveMetastoreExtension(Collections.emptyMap()); private static HiveCatalog catalog; @BeforeAll - public static void initCatalog() { + public static void initCatalog() throws TException { catalog = (HiveCatalog) CatalogUtil.loadCatalog( @@ -83,6 +85,9 @@ public static void initCatalog() { CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, String.valueOf(TimeUnit.SECONDS.toMillis(10))), HIVE_METASTORE_EXTENSION.hiveConf()); + String dbPath = HIVE_METASTORE_EXTENSION.metastore().getDatabasePath(DB_NAME); + Database db = new Database(DB_NAME, "description", dbPath, Maps.newHashMap()); + HIVE_METASTORE_EXTENSION.metastoreClient().createDatabase(db); } @BeforeEach diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreExtension.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreExtension.java index 552307395c59..57c97be32e23 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreExtension.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveMetastoreExtension.java @@ -21,8 +21,6 @@ import java.util.Map; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; -import org.apache.hadoop.hive.metastore.api.Database; -import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.ExtensionContext; @@ -31,10 +29,8 @@ public class HiveMetastoreExtension implements BeforeAllCallback, AfterAllCallba private HiveMetaStoreClient metastoreClient; private TestHiveMetastore metastore; private final Map hiveConfOverride; - private final String databaseName; - public HiveMetastoreExtension(String databaseName, Map hiveConfOverride) { - this.databaseName = databaseName; + public HiveMetastoreExtension(Map hiveConfOverride) { this.hiveConfOverride = hiveConfOverride; } @@ -50,9 +46,6 @@ public void beforeAll(ExtensionContext extensionContext) throws Exception { metastore.start(hiveConfWithOverrides); metastoreClient = new HiveMetaStoreClient(hiveConfWithOverrides); - String dbPath = metastore.getDatabasePath(databaseName); - Database db = new Database(databaseName, "description", dbPath, Maps.newHashMap()); - metastoreClient.createDatabase(db); } @Override diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableBaseTest.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableBaseTest.java index 9dd193271ad6..666243c03211 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableBaseTest.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableBaseTest.java @@ -31,6 +31,7 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hive.metastore.api.Database; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.CatalogUtil; import org.apache.iceberg.PartitionSpec; @@ -38,7 +39,9 @@ import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.types.Types; +import org.apache.thrift.TException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -52,7 +55,7 @@ public class HiveTableBaseTest { @RegisterExtension protected static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = - new HiveMetastoreExtension(DB_NAME, Collections.emptyMap()); + new HiveMetastoreExtension(Collections.emptyMap()); protected static HiveCatalog catalog; @@ -71,7 +74,7 @@ public class HiveTableBaseTest { private Path tableLocation; @BeforeAll - public static void initCatalog() { + public static void initCatalog() throws TException { catalog = (HiveCatalog) CatalogUtil.loadCatalog( @@ -81,6 +84,9 @@ public static void initCatalog() { CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, String.valueOf(TimeUnit.SECONDS.toMillis(10))), HIVE_METASTORE_EXTENSION.hiveConf()); + String dbPath = HIVE_METASTORE_EXTENSION.metastore().getDatabasePath(DB_NAME); + Database db = new Database(DB_NAME, "description", dbPath, Maps.newHashMap()); + HIVE_METASTORE_EXTENSION.metastoreClient().createDatabase(db); } @BeforeEach diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestCachedClientPool.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestCachedClientPool.java index 211fcdbd0c71..08be8adf7573 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestCachedClientPool.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestCachedClientPool.java @@ -40,11 +40,10 @@ public class TestCachedClientPool { private static final long EVICTION_INTERVAL = TimeUnit.SECONDS.toMillis(10); - private static final String DB_NAME = "hivedb"; @RegisterExtension private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = - new HiveMetastoreExtension(DB_NAME, Collections.emptyMap()); + new HiveMetastoreExtension(Collections.emptyMap()); @Test public void testClientPoolCleaner() throws InterruptedException { diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java index 49885cc3af2c..871ef6cdac72 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java @@ -18,8 +18,6 @@ */ package org.apache.iceberg.hive; -import static org.apache.iceberg.NullOrder.NULLS_FIRST; -import static org.apache.iceberg.SortDirection.ASC; import static org.apache.iceberg.TableProperties.CURRENT_SCHEMA; import static org.apache.iceberg.TableProperties.CURRENT_SNAPSHOT_ID; import static org.apache.iceberg.TableProperties.CURRENT_SNAPSHOT_SUMMARY; @@ -38,7 +36,6 @@ import java.io.IOException; import java.nio.file.Path; import java.util.Collections; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; @@ -54,22 +51,18 @@ import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; import org.apache.iceberg.FileFormat; -import org.apache.iceberg.HasTableOperations; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.PartitionSpecParser; import org.apache.iceberg.Schema; import org.apache.iceberg.SchemaParser; import org.apache.iceberg.Snapshot; -import org.apache.iceberg.SortOrder; -import org.apache.iceberg.SortOrderParser; import org.apache.iceberg.Table; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.TableOperations; import org.apache.iceberg.TableProperties; -import org.apache.iceberg.TestHelpers; -import org.apache.iceberg.Transaction; import org.apache.iceberg.UpdateSchema; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.CatalogTests; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; @@ -79,19 +72,19 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet; import org.apache.iceberg.relocated.com.google.common.collect.Maps; -import org.apache.iceberg.transforms.Transform; -import org.apache.iceberg.transforms.Transforms; import org.apache.iceberg.types.Types; import org.apache.iceberg.util.JsonUtil; import org.apache.thrift.TException; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; -public class TestHiveCatalog { +/** + * Run all the tests from abstract of {@link CatalogTests} with few specific tests related to HIVE. + */ +public class TestHiveCatalog extends CatalogTests { private static ImmutableMap meta = ImmutableMap.of( "owner", "apache", @@ -105,10 +98,10 @@ public class TestHiveCatalog { @RegisterExtension private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = - new HiveMetastoreExtension(DB_NAME, Collections.emptyMap()); + new HiveMetastoreExtension(Collections.emptyMap()); @BeforeEach - public void before() { + public void before() throws TException { catalog = (HiveCatalog) CatalogUtil.loadCatalog( @@ -118,44 +111,40 @@ public void before() { CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, String.valueOf(TimeUnit.SECONDS.toMillis(10))), HIVE_METASTORE_EXTENSION.hiveConf()); + String dbPath = HIVE_METASTORE_EXTENSION.metastore().getDatabasePath(DB_NAME); + Database db = new Database(DB_NAME, "description", dbPath, Maps.newHashMap()); + HIVE_METASTORE_EXTENSION.metastoreClient().createDatabase(db); } - private Schema getTestSchema() { - return new Schema( - required(1, "id", Types.IntegerType.get(), "unique ID"), - required(2, "data", Types.StringType.get())); + @AfterEach + public void cleanup() throws Exception { + HIVE_METASTORE_EXTENSION.metastore().reset(); } - @Test - public void testCreateTableBuilder() throws Exception { - Schema schema = getTestSchema(); - PartitionSpec spec = PartitionSpec.builderFor(schema).bucket("data", 16).build(); - TableIdentifier tableIdent = TableIdentifier.of(DB_NAME, "tbl"); - String location = temp.resolve("tbl").toString(); + @Override + protected boolean requiresNamespaceCreate() { + return true; + } - try { - Table table = - catalog - .buildTable(tableIdent, schema) - .withPartitionSpec(spec) - .withLocation(location) - .withProperty("key1", "value1") - .withProperty("key2", "value2") - .create(); + @Override + protected boolean supportsNamesWithSlashes() { + return false; + } - assertThat(table.location()).isEqualTo(location); - assertThat(table.schema().columns()).hasSize(2); - assertThat(table.spec().fields()).hasSize(1); - assertThat(table.properties()).containsEntry("key1", "value1"); - assertThat(table.properties()).containsEntry("key2", "value2"); - // default Parquet compression is explicitly set for new tables - assertThat(table.properties()) - .containsEntry( - TableProperties.PARQUET_COMPRESSION, - TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0); - } finally { - catalog.dropTable(tableIdent); - } + @Override + protected boolean supportsNamesWithDot() { + return false; + } + + @Override + protected HiveCatalog catalog() { + return catalog; + } + + private Schema getTestSchema() { + return new Schema( + required(1, "id", Types.IntegerType.get(), "unique ID"), + required(2, "data", Types.StringType.get())); } @Test @@ -219,81 +208,6 @@ public void testInitializeCatalogWithProperties() { .isEqualTo("/user/hive/testwarehouse"); } - @Test - public void testCreateTableTxnBuilder() throws Exception { - Schema schema = getTestSchema(); - TableIdentifier tableIdent = TableIdentifier.of(DB_NAME, "tbl"); - String location = temp.resolve("tbl").toString(); - - try { - Transaction txn = - catalog.buildTable(tableIdent, schema).withLocation(location).createTransaction(); - txn.commitTransaction(); - Table table = catalog.loadTable(tableIdent); - - assertThat(table.location()).isEqualTo(location); - assertThat(table.schema().columns()).hasSize(2); - assertThat(table.spec().isUnpartitioned()).isTrue(); - } finally { - catalog.dropTable(tableIdent); - } - } - - @ParameterizedTest - @ValueSource(ints = {1, 2}) - public void testReplaceTxnBuilder(int formatVersion) { - Schema schema = getTestSchema(); - PartitionSpec spec = PartitionSpec.builderFor(schema).bucket("data", 16).build(); - TableIdentifier tableIdent = TableIdentifier.of(DB_NAME, "tbl"); - String location = temp.resolve("tbl").toString(); - - try { - Transaction createTxn = - catalog - .buildTable(tableIdent, schema) - .withPartitionSpec(spec) - .withLocation(location) - .withProperty("key1", "value1") - .withProperty(TableProperties.FORMAT_VERSION, String.valueOf(formatVersion)) - .createOrReplaceTransaction(); - createTxn.commitTransaction(); - - Table table = catalog.loadTable(tableIdent); - assertThat(table.spec().fields()).hasSize(1); - - String newLocation = temp.resolve("tbl-2").toString(); - - Transaction replaceTxn = - catalog - .buildTable(tableIdent, schema) - .withProperty("key2", "value2") - .withLocation(newLocation) - .replaceTransaction(); - replaceTxn.commitTransaction(); - - table = catalog.loadTable(tableIdent); - assertThat(table.location()).isEqualTo(newLocation); - assertThat(table.currentSnapshot()).isNull(); - if (formatVersion == 1) { - PartitionSpec v1Expected = - PartitionSpec.builderFor(table.schema()) - .alwaysNull("data", "data_bucket") - .withSpecId(1) - .build(); - assertThat(table.spec()) - .as("Table should have a spec with one void field") - .isEqualTo(v1Expected); - } else { - assertThat(table.spec().isUnpartitioned()).as("Table spec must be unpartitioned").isTrue(); - } - - assertThat(table.properties()).containsEntry("key1", "value1"); - assertThat(table.properties()).containsEntry("key2", "value2"); - } finally { - catalog.dropTable(tableIdent); - } - } - @Test public void testCreateTableWithOwner() throws Exception { createTableAndVerifyOwner( @@ -328,59 +242,7 @@ private void createTableAndVerifyOwner( } @Test - public void testCreateTableDefaultSortOrder() throws Exception { - Schema schema = getTestSchema(); - PartitionSpec spec = PartitionSpec.builderFor(schema).bucket("data", 16).build(); - TableIdentifier tableIdent = TableIdentifier.of(DB_NAME, "tbl"); - - try { - Table table = catalog.createTable(tableIdent, schema, spec); - assertThat(table.sortOrder().orderId()).as("Order ID must match").isEqualTo(0); - assertThat(table.sortOrder().isUnsorted()).as("Order must unsorted").isTrue(); - - assertThat(hmsTableParameters()) - .as("Must not have default sort order in catalog") - .doesNotContainKey(DEFAULT_SORT_ORDER); - } finally { - catalog.dropTable(tableIdent); - } - } - - @Test - public void testCreateTableCustomSortOrder() throws Exception { - Schema schema = getTestSchema(); - PartitionSpec spec = PartitionSpec.builderFor(schema).bucket("data", 16).build(); - SortOrder order = SortOrder.builderFor(schema).asc("id", NULLS_FIRST).build(); - TableIdentifier tableIdent = TableIdentifier.of(DB_NAME, "tbl"); - - try { - Table table = - catalog - .buildTable(tableIdent, schema) - .withPartitionSpec(spec) - .withSortOrder(order) - .create(); - SortOrder sortOrder = table.sortOrder(); - assertThat(sortOrder.orderId()).as("Order ID must match").isEqualTo(1); - assertThat(sortOrder.fields()).as("Order must have 1 field").hasSize(1); - assertThat(sortOrder.fields().get(0).direction()).as("Direction must match ").isEqualTo(ASC); - assertThat(sortOrder.fields().get(0).nullOrder()) - .as("Null order must match ") - .isEqualTo(NULLS_FIRST); - Transform transform = Transforms.identity(Types.IntegerType.get()); - assertThat(sortOrder.fields().get(0).transform()) - .as("Transform must match") - .isEqualTo(transform); - - assertThat(hmsTableParameters()) - .containsEntry(DEFAULT_SORT_ORDER, SortOrderParser.toJson(table.sortOrder())); - } finally { - catalog.dropTable(tableIdent); - } - } - - @Test - public void testCreateNamespace() throws Exception { + public void testDatabaseAndNamespaceWithLocation() throws Exception { Namespace namespace1 = Namespace.of("noLocation"); catalog.createNamespace(namespace1, meta); Database database1 = @@ -395,7 +257,7 @@ public void testCreateNamespace() throws Exception { assertThatThrownBy(() -> catalog.createNamespace(namespace1)) .isInstanceOf(AlreadyExistsException.class) - .hasMessage("Namespace '" + namespace1 + "' already exists!"); + .hasMessage(String.format("Namespace already exists: %s", namespace1)); String hiveLocalDir = temp.toFile().toURI().toString(); // remove the trailing slash of the URI hiveLocalDir = hiveLocalDir.substring(0, hiveLocalDir.length() - 1); @@ -494,21 +356,6 @@ private void createNamespaceAndVerifyOwnership( assertThat(db.getOwnerType()).isEqualTo(expectedOwnerType); } - @Test - public void testListNamespace() throws TException { - List namespaces; - Namespace namespace1 = Namespace.of("dbname1"); - catalog.createNamespace(namespace1, meta); - namespaces = catalog.listNamespaces(namespace1); - assertThat(namespaces).as("Hive db not hive the namespace 'dbname1'").isEmpty(); - - Namespace namespace2 = Namespace.of("dbname2"); - catalog.createNamespace(namespace2, meta); - namespaces = catalog.listNamespaces(); - - assertThat(namespaces).as("Hive db not hive the namespace 'dbname2'").contains(namespace2); - } - @Test public void testLoadNamespaceMeta() throws TException { Namespace namespace = Namespace.of("dbname_load"); @@ -535,30 +382,6 @@ public void testNamespaceExists() throws TException { .isFalse(); } - @Test - public void testSetNamespaceProperties() throws TException { - Namespace namespace = Namespace.of("dbname_set"); - - catalog.createNamespace(namespace, meta); - catalog.setProperties( - namespace, - ImmutableMap.of( - "owner", "alter_apache", - "test", "test", - "location", "file:/data/tmp", - "comment", "iceberg test")); - - Database database = HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(namespace.level(0)); - assertThat(database.getParameters()).containsEntry("owner", "alter_apache"); - assertThat(database.getParameters()).containsEntry("test", "test"); - assertThat(database.getParameters()).containsEntry("group", "iceberg"); - - assertThatThrownBy( - () -> catalog.setProperties(Namespace.of("db2", "db2", "ns2"), ImmutableMap.of())) - .isInstanceOf(NoSuchNamespaceException.class) - .hasMessage("Namespace does not exist: db2.db2.ns2"); - } - @Test public void testSetNamespaceOwnership() throws TException { setNamespaceOwnershipAndVerify( @@ -740,27 +563,6 @@ private void setNamespaceOwnershipAndVerify( assertThat(database.getOwnerType()).isEqualTo(expectedOwnerTypePostSet); } - @Test - public void testRemoveNamespaceProperties() throws TException { - Namespace namespace = Namespace.of("dbname_remove"); - - catalog.createNamespace(namespace, meta); - - catalog.removeProperties(namespace, ImmutableSet.of("comment", "owner")); - - Database database = HIVE_METASTORE_EXTENSION.metastoreClient().getDatabase(namespace.level(0)); - - assertThat(database.getParameters()).doesNotContainKey("owner"); - assertThat(database.getParameters()).containsEntry("group", "iceberg"); - - assertThatThrownBy( - () -> - catalog.removeProperties( - Namespace.of("db2", "db2", "ns2"), ImmutableSet.of("comment", "owner"))) - .isInstanceOf(NoSuchNamespaceException.class) - .hasMessage("Namespace does not exist: db2.db2.ns2"); - } - @Test public void testRemoveNamespaceOwnership() throws TException, IOException { removeNamespaceOwnershipAndVerify( @@ -887,7 +689,7 @@ private void removeNamespaceOwnershipAndVerify( } @Test - public void testDropNamespace() throws TException { + public void dropNamespace() { Namespace namespace = Namespace.of("dbname_drop"); TableIdentifier identifier = TableIdentifier.of(namespace, "table"); Schema schema = getTestSchema(); @@ -1210,35 +1012,4 @@ public void testDatabaseLocationWithSlashInWarehouseDir() { assertThat(database.getLocationUri()).isEqualTo("s3://bucket/database.db"); } - - @Test - public void testRegisterTable() { - TableIdentifier identifier = TableIdentifier.of(DB_NAME, "t1"); - catalog.createTable(identifier, getTestSchema()); - Table registeringTable = catalog.loadTable(identifier); - catalog.dropTable(identifier, false); - TableOperations ops = ((HasTableOperations) registeringTable).operations(); - String metadataLocation = ((HiveTableOperations) ops).currentMetadataLocation(); - Table registeredTable = catalog.registerTable(identifier, metadataLocation); - assertThat(registeredTable).isNotNull(); - TestHelpers.assertSerializedAndLoadedMetadata(registeringTable, registeredTable); - String expectedMetadataLocation = - ((HasTableOperations) registeredTable).operations().current().metadataFileLocation(); - assertThat(metadataLocation).isEqualTo(expectedMetadataLocation); - assertThat(catalog.loadTable(identifier)).isNotNull(); - assertThat(catalog.dropTable(identifier)).isTrue(); - } - - @Test - public void testRegisterExistingTable() { - TableIdentifier identifier = TableIdentifier.of(DB_NAME, "t1"); - catalog.createTable(identifier, getTestSchema()); - Table registeringTable = catalog.loadTable(identifier); - TableOperations ops = ((HasTableOperations) registeringTable).operations(); - String metadataLocation = ((HiveTableOperations) ops).currentMetadataLocation(); - assertThatThrownBy(() -> catalog.registerTable(identifier, metadataLocation)) - .isInstanceOf(AlreadyExistsException.class) - .hasMessage("Table already exists: hivedb.t1"); - assertThat(catalog.dropTable(identifier, true)).isTrue(); - } } diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommitLocks.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommitLocks.java index 48946525463c..92070cc544a5 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommitLocks.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommitLocks.java @@ -50,6 +50,7 @@ import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.Database; import org.apache.hadoop.hive.metastore.api.EnvironmentContext; import org.apache.hadoop.hive.metastore.api.LockRequest; import org.apache.hadoop.hive.metastore.api.LockResponse; @@ -68,6 +69,7 @@ import org.apache.iceberg.hadoop.ConfigProperties; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.types.Types; import org.apache.thrift.TException; import org.junit.jupiter.api.AfterAll; @@ -107,8 +109,7 @@ public class TestHiveCommitLocks { @RegisterExtension private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = - new HiveMetastoreExtension( - DB_NAME, ImmutableMap.of(HiveConf.ConfVars.HIVE_TXN_TIMEOUT.varname, "1s")); + new HiveMetastoreExtension(ImmutableMap.of(HiveConf.ConfVars.HIVE_TXN_TIMEOUT.varname, "1s")); private static HiveCatalog catalog; private Path tableLocation; @@ -124,6 +125,9 @@ public static void initCatalog() throws Exception { CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS, String.valueOf(TimeUnit.SECONDS.toMillis(10))), HIVE_METASTORE_EXTENSION.hiveConf()); + String dbPath = HIVE_METASTORE_EXTENSION.metastore().getDatabasePath(DB_NAME); + Database db = new Database(DB_NAME, "description", dbPath, Maps.newHashMap()); + HIVE_METASTORE_EXTENSION.metastoreClient().createDatabase(db); // start spies overriddenHiveConf = new Configuration(HIVE_METASTORE_EXTENSION.hiveConf());