Skip to content

Commit

Permalink
Hive: Refactor TestHiveCatalog tests to use the core CatalogTests
Browse files Browse the repository at this point in the history
  • Loading branch information
@nk1506
nk1506 committed Dec 13, 2023
1 parent 7240752 commit e95be19
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 97 deletions.
1 change: 1 addition & 0 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
8 changes: 8 additions & 0 deletions core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ protected boolean supportsNamesWithSlashes() {
return true;
}

protected boolean supportsNamesWithDot() {
return true;
}

@Test
public void testCreateNamespace() {
C catalog = catalog();
Expand Down Expand Up @@ -470,6 +474,8 @@ public void testNamespaceWithSlash() {

@Test
public void testNamespaceWithDot() {
Assumptions.assumeTrue(supportsNamesWithDot());

C catalog = catalog();

Namespace withDot = Namespace.of("new.db");
Expand Down Expand Up @@ -547,6 +553,8 @@ public void testTableNameWithSlash() {

@Test
public void testTableNameWithDot() {
Assumptions.assumeTrue(supportsNamesWithDot());

C catalog = catalog();

TableIdentifier ident = TableIdentifier.of("ns", "ta.ble");
Expand Down
5 changes: 4 additions & 1 deletion hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ public void createNamespace(Namespace namespace, Map<String, String> 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(
Expand Down Expand Up @@ -500,6 +500,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);
Expand Down
2 changes: 1 addition & 1 deletion hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
177 changes: 82 additions & 95 deletions hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -65,10 +64,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;
Expand All @@ -83,14 +82,19 @@
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.JsonUtil;
import org.apache.thrift.TException;
import org.assertj.core.api.Assertions;
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<HiveCatalog> {
private static ImmutableMap meta =
ImmutableMap.of(
"owner", "apache",
Expand All @@ -104,10 +108,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(
Expand All @@ -117,6 +121,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() {
Expand Down Expand Up @@ -379,7 +411,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 =
Expand All @@ -394,7 +426,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);
Expand Down Expand Up @@ -493,21 +525,6 @@ private void createNamespaceAndVerifyOwnership(
assertThat(db.getOwnerType()).isEqualTo(expectedOwnerType);
}

@Test
public void testListNamespace() throws TException {
List<Namespace> 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");
Expand All @@ -534,30 +551,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(
Expand Down Expand Up @@ -739,27 +732,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(
Expand Down Expand Up @@ -886,7 +858,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();
Expand Down Expand Up @@ -1211,33 +1183,48 @@ public void testDatabaseLocationWithSlashInWarehouseDir() {
}

@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();
}
@Override
public void testRenameTableDestinationTableAlreadyExists() {
Namespace ns = Namespace.of("newdb");
TableIdentifier renamedTable = TableIdentifier.of(ns, "table_renamed");

@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();
if (requiresNamespaceCreate()) {
catalog.createNamespace(ns);
}

Assertions.assertThat(catalog.tableExists(TABLE))
.as("Source table should not exist before create")
.isFalse();

catalog.buildTable(TABLE, SCHEMA).create();
Assertions.assertThat(catalog.tableExists(TABLE))
.as("Source table should exist after create")
.isTrue();

Assertions.assertThat(catalog.tableExists(renamedTable))
.as("Destination table should not exist before create")
.isFalse();

catalog.buildTable(renamedTable, SCHEMA).create();
Assertions.assertThat(catalog.tableExists(renamedTable))
.as("Destination table should exist after create")
.isTrue();
Assertions.assertThatThrownBy(() -> catalog.renameTable(TABLE, renamedTable))
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("new table newdb.table_renamed already exists");
Assertions.assertThat(catalog.tableExists(TABLE))
.as("Source table should still exist after failed rename")
.isTrue();
Assertions.assertThat(catalog.tableExists(renamedTable))
.as("Destination table should still exist after failed rename")
.isTrue();

String sourceTableUUID =
((HasTableOperations) catalog.loadTable(TABLE)).operations().current().uuid();
String destinationTableUUID =
((HasTableOperations) catalog.loadTable(renamedTable)).operations().current().uuid();
Assertions.assertThat(sourceTableUUID)
.as("Source and destination table should remain distinct after failed rename")
.isNotEqualTo(destinationTableUUID);
}
}

0 comments on commit e95be19

Please sign in to comment.