This is an automated email from the ASF dual-hosted git repository.
etudenhoefner 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 fab5e18d0d Hive, JDBC: Avoid NPE on Throwables without error msg
(#10082)
fab5e18d0d is described below
commit fab5e18d0db3b6a05ada5e4c2fb9728e21bca35f
Author: Naveen Kumar <[email protected]>
AuthorDate: Fri Apr 5 17:34:03 2024 +0530
Hive, JDBC: Avoid NPE on Throwables without error msg (#10082)
---
.../apache/iceberg/jdbc/JdbcTableOperations.java | 2 +-
.../apache/iceberg/jdbc/JdbcViewOperations.java | 2 +-
.../org/apache/iceberg/jdbc/TestJdbcCatalog.java | 45 +++++++++++++++
.../apache/iceberg/jdbc/TestJdbcViewCatalog.java | 67 ++++++++++++++++++++++
.../org/apache/iceberg/hive/HiveClientPool.java | 3 +-
.../apache/iceberg/hive/TestHiveClientPool.java | 45 +++++++++++++++
6 files changed, 161 insertions(+), 3 deletions(-)
diff --git
a/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java
b/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java
index 68d75b8e4f..619296ad33 100644
--- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java
+++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java
@@ -138,7 +138,7 @@ class JdbcTableOperations extends
BaseMetastoreTableOperations {
throw new UncheckedSQLException(e, "Database warning");
} catch (SQLException e) {
// SQLite doesn't set SQLState or throw
SQLIntegrityConstraintViolationException
- if (e.getMessage().contains("constraint failed")) {
+ if (e.getMessage() != null && e.getMessage().contains("constraint
failed")) {
throw new AlreadyExistsException("Table already exists: %s",
tableIdentifier);
}
diff --git a/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java
b/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java
index 2ded12b102..10f46941d6 100644
--- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java
+++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java
@@ -129,7 +129,7 @@ public class JdbcViewOperations extends BaseViewOperations {
throw new UncheckedSQLException(e, "Database warning");
} catch (SQLException e) {
// SQLite doesn't set SQLState or throw
SQLIntegrityConstraintViolationException
- if (e.getMessage().contains("constraint failed")) {
+ if (e.getMessage() != null && e.getMessage().contains("constraint
failed")) {
throw new AlreadyExistsException("View already exists: %s",
viewIdentifier);
}
diff --git a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java
b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java
index 48ad717347..d8553a1858 100644
--- a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java
+++ b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java
@@ -23,6 +23,7 @@ import static org.apache.iceberg.SortDirection.ASC;
import static org.apache.iceberg.types.Types.NestedField.required;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
import java.io.File;
import java.io.IOException;
@@ -41,6 +42,7 @@ import java.util.stream.Stream;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.iceberg.BaseTable;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.CatalogUtil;
import org.apache.iceberg.DataFile;
@@ -51,6 +53,7 @@ import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.SortOrder;
import org.apache.iceberg.Table;
+import org.apache.iceberg.TableMetadata;
import org.apache.iceberg.TableOperations;
import org.apache.iceberg.TableProperties;
import org.apache.iceberg.Transaction;
@@ -79,6 +82,8 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
import org.sqlite.SQLiteDataSource;
public class TestJdbcCatalog extends CatalogTests<JdbcCatalog> {
@@ -970,6 +975,46 @@ public class TestJdbcCatalog extends
CatalogTests<JdbcCatalog> {
Assertions.assertThat(CustomMetricsReporter.COUNTER.get()).isEqualTo(2);
}
+ @Test
+ public void testCommitExceptionWithoutMessage() {
+ TableIdentifier tableIdent = TableIdentifier.of("db", "tbl");
+ BaseTable table = (BaseTable) catalog.buildTable(tableIdent,
SCHEMA).create();
+ TableOperations ops = table.operations();
+ TableMetadata metadataV1 = ops.current();
+
+ table.updateSchema().addColumn("n", Types.IntegerType.get()).commit();
+ ops.refresh();
+
+ try (MockedStatic<JdbcUtil> mockedStatic =
Mockito.mockStatic(JdbcUtil.class)) {
+ mockedStatic
+ .when(() -> JdbcUtil.loadTable(any(), any(), any(), any()))
+ .thenThrow(new SQLException());
+ assertThatThrownBy(() -> ops.commit(ops.current(), metadataV1))
+ .isInstanceOf(UncheckedSQLException.class)
+ .hasMessageStartingWith("Unknown failure");
+ }
+ }
+
+ @Test
+ public void testCommitExceptionWithMessage() {
+ TableIdentifier tableIdent = TableIdentifier.of("db", "tbl");
+ BaseTable table = (BaseTable) catalog.buildTable(tableIdent,
SCHEMA).create();
+ TableOperations ops = table.operations();
+ TableMetadata metadataV1 = ops.current();
+
+ table.updateSchema().addColumn("n", Types.IntegerType.get()).commit();
+ ops.refresh();
+
+ try (MockedStatic<JdbcUtil> mockedStatic =
Mockito.mockStatic(JdbcUtil.class)) {
+ mockedStatic
+ .when(() -> JdbcUtil.loadTable(any(), any(), any(), any()))
+ .thenThrow(new SQLException("constraint failed"));
+ assertThatThrownBy(() -> ops.commit(ops.current(), metadataV1))
+ .isInstanceOf(AlreadyExistsException.class)
+ .hasMessageStartingWith("Table already exists: " + tableIdent);
+ }
+ }
+
public static class CustomMetricsReporter implements MetricsReporter {
static final AtomicInteger COUNTER = new AtomicInteger(0);
diff --git
a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java
b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java
index 8c02557642..a66532d90f 100644
--- a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java
+++ b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcViewCatalog.java
@@ -18,15 +18,28 @@
*/
package org.apache.iceberg.jdbc;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+
+import java.sql.SQLException;
import java.util.Map;
import java.util.UUID;
import org.apache.hadoop.conf.Configuration;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.view.BaseView;
import org.apache.iceberg.view.ViewCatalogTests;
+import org.apache.iceberg.view.ViewMetadata;
+import org.apache.iceberg.view.ViewOperations;
import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
public class TestJdbcViewCatalog extends ViewCatalogTests<JdbcCatalog> {
@@ -64,4 +77,58 @@ public class TestJdbcViewCatalog extends
ViewCatalogTests<JdbcCatalog> {
protected boolean requiresNamespaceCreate() {
return true;
}
+
+ @Test
+ public void testCommitExceptionWithoutMessage() {
+ TableIdentifier identifier = TableIdentifier.of("namespace1", "view");
+ BaseView view =
+ (BaseView)
+ catalog
+ .buildView(identifier)
+ .withQuery("spark", "select * from tbl")
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(Namespace.of("namespace1"))
+ .create();
+ ViewOperations ops = view.operations();
+ ViewMetadata metadataV1 = ops.current();
+
+ view.updateProperties().set("k1", "v1").commit();
+ ops.refresh();
+
+ try (MockedStatic<JdbcUtil> mockedStatic =
Mockito.mockStatic(JdbcUtil.class)) {
+ mockedStatic
+ .when(() -> JdbcUtil.loadView(any(), any(), any(), any()))
+ .thenThrow(new SQLException());
+ assertThatThrownBy(() -> ops.commit(ops.current(), metadataV1))
+ .isInstanceOf(UncheckedSQLException.class)
+ .hasMessageStartingWith("Unknown failure");
+ }
+ }
+
+ @Test
+ public void testCommitExceptionWithMessage() {
+ TableIdentifier identifier = TableIdentifier.of("namespace1", "view");
+ BaseView view =
+ (BaseView)
+ catalog
+ .buildView(identifier)
+ .withQuery("spark", "select * from tbl")
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(Namespace.of("namespace1"))
+ .create();
+ ViewOperations ops = view.operations();
+ ViewMetadata metadataV1 = ops.current();
+
+ view.updateProperties().set("k1", "v1").commit();
+ ops.refresh();
+
+ try (MockedStatic<JdbcUtil> mockedStatic =
Mockito.mockStatic(JdbcUtil.class)) {
+ mockedStatic
+ .when(() -> JdbcUtil.loadView(any(), any(), any(), any()))
+ .thenThrow(new SQLException("constraint failed"));
+ assertThatThrownBy(() -> ops.commit(ops.current(), metadataV1))
+ .isInstanceOf(AlreadyExistsException.class)
+ .hasMessageStartingWith("View already exists: " + identifier);
+ }
+ }
}
diff --git
a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveClientPool.java
b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveClientPool.java
index 9bc232043a..b0ecb0ceff 100644
--- a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveClientPool.java
+++ b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveClientPool.java
@@ -73,7 +73,8 @@ public class HiveClientPool extends
ClientPoolImpl<IMetaStoreClient, TException>
} catch (MetaException e) {
throw new RuntimeMetaException(e, "Failed to connect to Hive Metastore");
} catch (Throwable t) {
- if (t.getMessage().contains("Another instance of Derby may have already
booted")) {
+ if (t.getMessage() != null
+ && t.getMessage().contains("Another instance of Derby may have
already booted")) {
throw new RuntimeMetaException(
t,
"Failed to start an embedded metastore because embedded "
diff --git
a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
index 5a565d0e98..2fe1bacf9d 100644
---
a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
+++
b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
@@ -20,6 +20,7 @@ package org.apache.iceberg.hive;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
import java.io.ByteArrayInputStream;
import java.io.IOException;
@@ -29,6 +30,7 @@ import java.util.List;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hive.conf.HiveConf;
import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
+import org.apache.hadoop.hive.metastore.MetaStoreUtils;
import org.apache.hadoop.hive.metastore.api.Function;
import org.apache.hadoop.hive.metastore.api.FunctionType;
import org.apache.hadoop.hive.metastore.api.GetAllFunctionsResponse;
@@ -39,6 +41,7 @@ import org.apache.thrift.transport.TTransportException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
import org.mockito.Mockito;
public class TestHiveClientPool {
@@ -116,6 +119,48 @@ public class TestHiveClientPool {
.hasMessage("Another meta exception");
}
+ @Test
+ public void testExceptionMessages() {
+ try (MockedStatic<MetaStoreUtils> mockedStatic =
Mockito.mockStatic(MetaStoreUtils.class)) {
+ mockedStatic
+ .when(() -> MetaStoreUtils.newInstance(any(), any(), any()))
+ .thenThrow(new RuntimeException(new MetaException("Another meta
exception")));
+ assertThatThrownBy(() -> clients.run(client ->
client.getTables("default", "t")))
+ .isInstanceOf(RuntimeMetaException.class)
+ .hasMessage("Failed to connect to Hive Metastore");
+ }
+
+ try (MockedStatic<MetaStoreUtils> mockedStatic =
Mockito.mockStatic(MetaStoreUtils.class)) {
+ mockedStatic
+ .when(() -> MetaStoreUtils.newInstance(any(), any(), any()))
+ .thenThrow(new RuntimeException(new MetaException()));
+ assertThatThrownBy(() -> clients.run(client ->
client.getTables("default", "t")))
+ .isInstanceOf(RuntimeMetaException.class)
+ .hasMessage("Failed to connect to Hive Metastore");
+ }
+
+ try (MockedStatic<MetaStoreUtils> mockedStatic =
Mockito.mockStatic(MetaStoreUtils.class)) {
+ mockedStatic
+ .when(() -> MetaStoreUtils.newInstance(any(), any(), any()))
+ .thenThrow(new RuntimeException());
+ assertThatThrownBy(() -> clients.run(client ->
client.getTables("default", "t")))
+ .isInstanceOf(RuntimeMetaException.class)
+ .hasMessage("Failed to connect to Hive Metastore");
+ }
+
+ try (MockedStatic<MetaStoreUtils> mockedStatic =
Mockito.mockStatic(MetaStoreUtils.class)) {
+ mockedStatic
+ .when(() -> MetaStoreUtils.newInstance(any(), any(), any()))
+ .thenThrow(new RuntimeException("Another instance of Derby may have
already booted"));
+ assertThatThrownBy(() -> clients.run(client ->
client.getTables("default", "t")))
+ .isInstanceOf(RuntimeMetaException.class)
+ .hasMessage(
+ "Failed to start an embedded metastore because embedded "
+ + "Derby supports only one client at a time. To fix this,
use a metastore that supports "
+ + "multiple clients.");
+ }
+ }
+
@Test
public void testConnectionFailureRestoreForMetaException() throws Exception {
HiveMetaStoreClient hmsClient = newClient();
