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/TestHiveCatalog.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java index b70cb1c8e5b1..369ad46c8e49 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 @@ -37,7 +37,6 @@ import java.io.IOException; import java.nio.file.Path; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; @@ -53,7 +52,6 @@ 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; @@ -65,10 +63,10 @@ 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; @@ -83,6 +81,7 @@ 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; @@ -90,7 +89,10 @@ 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", @@ -104,10 +106,10 @@ public class TestHiveCatalog { @RegisterExtension private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION = - HiveMetastoreExtension.builder().withDatabase(DB_NAME).build(); + HiveMetastoreExtension.builder().build(); @BeforeEach - public void before() { + public void before() throws TException { catalog = (HiveCatalog) CatalogUtil.loadCatalog( @@ -117,6 +119,34 @@ 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); + } + + @AfterEach + public void cleanup() throws Exception { + HIVE_METASTORE_EXTENSION.metastore().reset(); + } + + @Override + protected boolean requiresNamespaceCreate() { + return true; + } + + @Override + protected boolean supportsNamesWithSlashes() { + return false; + } + + @Override + protected boolean supportsNamesWithDot() { + return false; + } + + @Override + protected HiveCatalog catalog() { + return catalog; } private Schema getTestSchema() { @@ -379,7 +409,7 @@ public void testCreateTableCustomSortOrder() throws Exception { } @Test - public void testCreateNamespace() throws Exception { + public void testDatabaseAndNamespaceWithLocation() throws Exception { Namespace namespace1 = Namespace.of("noLocation"); catalog.createNamespace(namespace1, meta); Database database1 = @@ -394,7 +424,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); @@ -493,21 +523,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"); @@ -534,30 +549,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( @@ -739,27 +730,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( @@ -886,7 +856,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(); @@ -1209,35 +1179,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(); - } }