This is an automated email from the ASF dual-hosted git repository.
pvary pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 1da80552c0 Hive: Unwrap RuntimeException for Hive TException with
alter table (#9432)
1da80552c0 is described below
commit 1da80552c06e749f2b6103f0ab0a184bb77a841c
Author: Naveen Kumar <[email protected]>
AuthorDate: Wed Jan 17 15:44:21 2024 +0530
Hive: Unwrap RuntimeException for Hive TException with alter table (#9432)
---
.../java/org/apache/iceberg/hive/HiveCatalog.java | 11 +++--
.../org/apache/iceberg/hive/MetastoreUtil.java | 17 +++++--
.../org/apache/iceberg/hive/TestHiveCatalog.java | 53 ----------------------
3 files changed, 22 insertions(+), 59 deletions(-)
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 516483e49a..63e4aad5d8 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
@@ -251,9 +251,14 @@ public class HiveCatalog extends BaseMetastoreCatalog
implements SupportsNamespa
} catch (NoSuchObjectException e) {
throw new NoSuchTableException("Table does not exist: %s", from);
- } catch (AlreadyExistsException e) {
- throw new org.apache.iceberg.exceptions.AlreadyExistsException(
- "Table already exists: %s", to);
+ } catch (InvalidOperationException e) {
+ if (e.getMessage() != null
+ && e.getMessage().contains(String.format("new table %s already
exists", to))) {
+ throw new org.apache.iceberg.exceptions.AlreadyExistsException(
+ "Table already exists: %s", to);
+ } else {
+ throw new RuntimeException("Failed to rename " + from + " to " + to,
e);
+ }
} catch (TException e) {
throw new RuntimeException("Failed to rename " + from + " to " + to, e);
diff --git
a/hive-metastore/src/main/java/org/apache/iceberg/hive/MetastoreUtil.java
b/hive-metastore/src/main/java/org/apache/iceberg/hive/MetastoreUtil.java
index 83ff2b60d0..ef1dffb6ec 100644
--- a/hive-metastore/src/main/java/org/apache/iceberg/hive/MetastoreUtil.java
+++ b/hive-metastore/src/main/java/org/apache/iceberg/hive/MetastoreUtil.java
@@ -26,6 +26,7 @@ import org.apache.hadoop.hive.metastore.api.Table;
import org.apache.iceberg.common.DynMethods;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.thrift.TException;
public class MetastoreUtil {
private static final DynMethods.UnboundMethod ALTER_TABLE =
@@ -54,7 +55,7 @@ public class MetastoreUtil {
* context will be set in a way that turns off stats updates to avoid
recursive file listing.
*/
public static void alterTable(
- IMetaStoreClient client, String databaseName, String tblName, Table
table) {
+ IMetaStoreClient client, String databaseName, String tblName, Table
table) throws TException {
alterTable(client, databaseName, tblName, table, ImmutableMap.of());
}
@@ -67,11 +68,21 @@ public class MetastoreUtil {
String databaseName,
String tblName,
Table table,
- Map<String, String> extraEnv) {
+ Map<String, String> extraEnv)
+ throws TException {
Map<String, String> env = Maps.newHashMapWithExpectedSize(extraEnv.size()
+ 1);
env.putAll(extraEnv);
env.put(StatsSetupConst.DO_NOT_UPDATE_STATS, StatsSetupConst.TRUE);
- ALTER_TABLE.invoke(client, databaseName, tblName, table, new
EnvironmentContext(env));
+ try {
+ ALTER_TABLE.invoke(client, databaseName, tblName, table, new
EnvironmentContext(env));
+ } catch (RuntimeException e) {
+ // TException would be wrapped into RuntimeException during reflection
+ if (e.getCause() instanceof TException) {
+ throw (TException) e.getCause();
+ } else {
+ throw e;
+ }
+ }
}
}
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 904e74939e..369ad46c8e 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
@@ -52,7 +52,6 @@ import org.apache.iceberg.CatalogUtil;
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;
@@ -82,7 +81,6 @@ 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.assertj.core.api.Assertions;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -1181,55 +1179,4 @@ public class TestHiveCatalog extends
CatalogTests<HiveCatalog> {
assertThat(database.getLocationUri()).isEqualTo("s3://bucket/database.db");
}
-
- // TODO: This test should be removed after fix of
https://github.com/apache/iceberg/issues/9289.
- @Test
- @Override
- public void testRenameTableDestinationTableAlreadyExists() {
- Namespace ns = Namespace.of("newdb");
- TableIdentifier renamedTable = TableIdentifier.of(ns, "table_renamed");
-
- 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();
-
- // With fix of issues#9289,it should match with CatalogTests and expect
- // AlreadyExistsException.class
- // and message should contain as "Table already exists"
- 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);
- }
}
