From cfb560beaf72bb29b73c2a848918dddfd418a390 Mon Sep 17 00:00:00 2001 From: Dennis Huo Date: Tue, 13 Dec 2022 22:50:02 -0800 Subject: [PATCH 1/2] Add JdbcSnowflakeClientTest using mocks; provides full coverage of JdbcSnowflakeClient and entities' ResultSetHandler logic. --- .../snowflake/JdbcSnowflakeClient.java | 19 +- .../snowflake/entities/SnowflakeSchema.java | 24 ++ .../snowflake/entities/SnowflakeTable.java | 25 ++ .../entities/SnowflakeTableMetadata.java | 34 +- .../snowflake/JdbcSnowflakeClientTest.java | 395 ++++++++++++++++++ 5 files changed, 489 insertions(+), 8 deletions(-) create mode 100644 snowflake/src/test/java/org/apache/iceberg/snowflake/JdbcSnowflakeClientTest.java diff --git a/snowflake/src/main/java/org/apache/iceberg/snowflake/JdbcSnowflakeClient.java b/snowflake/src/main/java/org/apache/iceberg/snowflake/JdbcSnowflakeClient.java index 9730a5f3724b..d2870a94df18 100644 --- a/snowflake/src/main/java/org/apache/iceberg/snowflake/JdbcSnowflakeClient.java +++ b/snowflake/src/main/java/org/apache/iceberg/snowflake/JdbcSnowflakeClient.java @@ -26,6 +26,7 @@ import org.apache.iceberg.jdbc.JdbcClientPool; import org.apache.iceberg.jdbc.UncheckedInterruptedException; import org.apache.iceberg.jdbc.UncheckedSQLException; +import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting; import org.apache.iceberg.snowflake.entities.SnowflakeSchema; import org.apache.iceberg.snowflake.entities.SnowflakeTable; import org.apache.iceberg.snowflake.entities.SnowflakeTableMetadata; @@ -47,11 +48,17 @@ public class JdbcSnowflakeClient implements SnowflakeClient { private static final Logger LOG = LoggerFactory.getLogger(JdbcSnowflakeClient.class); private final JdbcClientPool connectionPool; + private QueryRunner queryRunner = new QueryRunner(true); JdbcSnowflakeClient(JdbcClientPool conn) { connectionPool = conn; } + @VisibleForTesting + void setQueryRunner(QueryRunner queryRunner) { + this.queryRunner = queryRunner; + } + @Override public List listSchemas(Namespace namespace) { StringBuilder baseQuery = new StringBuilder("SHOW SCHEMAS"); @@ -68,13 +75,13 @@ public List listSchemas(Namespace namespace) { final String finalQuery = baseQuery.toString(); final Object[] finalQueryParams = queryParams; - QueryRunner run = new QueryRunner(true); List schemas; try { schemas = connectionPool.run( conn -> - run.query(conn, finalQuery, SnowflakeSchema.createHandler(), finalQueryParams)); + queryRunner.query( + conn, finalQuery, SnowflakeSchema.createHandler(), finalQueryParams)); } catch (SQLException e) { throw new UncheckedSQLException( e, @@ -111,13 +118,13 @@ public List listIcebergTables(Namespace namespace) { final String finalQuery = baseQuery.toString(); final Object[] finalQueryParams = queryParams; - QueryRunner run = new QueryRunner(true); List tables; try { tables = connectionPool.run( conn -> - run.query(conn, finalQuery, SnowflakeTable.createHandler(), finalQueryParams)); + queryRunner.query( + conn, finalQuery, SnowflakeTable.createHandler(), finalQueryParams)); } catch (SQLException e) { throw new UncheckedSQLException( e, "Failed to list tables for namespace %s", namespace.toString()); @@ -129,15 +136,13 @@ public List listIcebergTables(Namespace namespace) { @Override public SnowflakeTableMetadata getTableMetadata(TableIdentifier tableIdentifier) { - QueryRunner run = new QueryRunner(true); - SnowflakeTableMetadata tableMeta; try { final String finalQuery = "SELECT SYSTEM$GET_ICEBERG_TABLE_INFORMATION(?) AS METADATA"; tableMeta = connectionPool.run( conn -> - run.query( + queryRunner.query( conn, finalQuery, SnowflakeTableMetadata.createHandler(), diff --git a/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeSchema.java b/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeSchema.java index 50410555ad48..b8acccff6f54 100644 --- a/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeSchema.java +++ b/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeSchema.java @@ -20,6 +20,7 @@ import java.util.List; import org.apache.commons.dbutils.ResultSetHandler; +import org.apache.iceberg.relocated.com.google.common.base.Objects; import org.apache.iceberg.relocated.com.google.common.collect.Lists; public class SnowflakeSchema { @@ -39,6 +40,29 @@ public String getDatabase() { return databaseName; } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } else if (!(o instanceof SnowflakeSchema)) { + return false; + } + + SnowflakeSchema that = (SnowflakeSchema) o; + return Objects.equal(this.databaseName, that.databaseName) + && Objects.equal(this.name, that.name); + } + + @Override + public int hashCode() { + return Objects.hashCode(databaseName, name); + } + + @Override + public String toString() { + return String.format("%s.%s", databaseName, name); + } + public static ResultSetHandler> createHandler() { return rs -> { List schemas = Lists.newArrayList(); diff --git a/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeTable.java b/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeTable.java index f619ed0ca7fa..fbb8ecd5cac5 100644 --- a/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeTable.java +++ b/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeTable.java @@ -20,6 +20,7 @@ import java.util.List; import org.apache.commons.dbutils.ResultSetHandler; +import org.apache.iceberg.relocated.com.google.common.base.Objects; import org.apache.iceberg.relocated.com.google.common.collect.Lists; public class SnowflakeTable { @@ -45,6 +46,30 @@ public String getSchemaName() { return schemaName; } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } else if (!(o instanceof SnowflakeTable)) { + return false; + } + + SnowflakeTable that = (SnowflakeTable) o; + return Objects.equal(this.databaseName, that.databaseName) + && Objects.equal(this.schemaName, that.schemaName) + && Objects.equal(this.name, that.name); + } + + @Override + public int hashCode() { + return Objects.hashCode(databaseName, schemaName, name); + } + + @Override + public String toString() { + return String.format("%s.%s.%s", databaseName, schemaName, name); + } + public static ResultSetHandler> createHandler() { return rs -> { List tables = Lists.newArrayList(); diff --git a/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeTableMetadata.java b/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeTableMetadata.java index 554b7db3bab4..d58bc81e3f73 100644 --- a/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeTableMetadata.java +++ b/snowflake/src/main/java/org/apache/iceberg/snowflake/entities/SnowflakeTableMetadata.java @@ -23,6 +23,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.commons.dbutils.ResultSetHandler; +import org.apache.iceberg.relocated.com.google.common.base.Objects; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.util.JsonUtil; @@ -31,9 +32,12 @@ public class SnowflakeTableMetadata { Pattern.compile("azure://([^/]+)/([^/]+)/(.*)"); private String snowflakeMetadataLocation; - private String status; private String icebergMetadataLocation; + private String status; + // Note: Since not all sources will necessarily come from a raw JSON representation, this raw + // JSON should only be considered a convenient debugging field. Equality of two + // SnowflakeTableMetadata instances should not depend on equality of this field. private String rawJsonVal; public SnowflakeTableMetadata( @@ -61,6 +65,34 @@ public String getStatus() { return status; } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } else if (!(o instanceof SnowflakeTableMetadata)) { + return false; + } + + // Only consider parsed fields, not the raw JSON that may or may not be the original source of + // this instance. + SnowflakeTableMetadata that = (SnowflakeTableMetadata) o; + return Objects.equal(this.snowflakeMetadataLocation, that.snowflakeMetadataLocation) + && Objects.equal(this.icebergMetadataLocation, that.icebergMetadataLocation) + && Objects.equal(this.status, that.status); + } + + @Override + public int hashCode() { + return Objects.hashCode(snowflakeMetadataLocation, icebergMetadataLocation, status); + } + + @Override + public String toString() { + return String.format( + "snowflakeMetadataLocation: '%s', icebergMetadataLocation: '%s', status: '%s", + snowflakeMetadataLocation, icebergMetadataLocation, status); + } + /** * Translates from Snowflake's path syntax to Iceberg's path syntax for paths matching known * non-compatible Snowflake paths. Throws IllegalArgumentException if the prefix of the diff --git a/snowflake/src/test/java/org/apache/iceberg/snowflake/JdbcSnowflakeClientTest.java b/snowflake/src/test/java/org/apache/iceberg/snowflake/JdbcSnowflakeClientTest.java new file mode 100644 index 000000000000..2757dbdf5989 --- /dev/null +++ b/snowflake/src/test/java/org/apache/iceberg/snowflake/JdbcSnowflakeClientTest.java @@ -0,0 +1,395 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.snowflake; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.List; +import org.apache.commons.dbutils.QueryRunner; +import org.apache.commons.dbutils.ResultSetHandler; +import org.apache.iceberg.ClientPool; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.jdbc.JdbcClientPool; +import org.apache.iceberg.jdbc.UncheckedInterruptedException; +import org.apache.iceberg.jdbc.UncheckedSQLException; +import org.apache.iceberg.relocated.com.google.common.collect.Lists; +import org.apache.iceberg.snowflake.entities.SnowflakeSchema; +import org.apache.iceberg.snowflake.entities.SnowflakeTable; +import org.apache.iceberg.snowflake.entities.SnowflakeTableMetadata; +import org.assertj.core.api.Assertions; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentMatchers; +import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; + +@RunWith(MockitoJUnitRunner.class) +public class JdbcSnowflakeClientTest { + @Mock private Connection mockConnection; + @Mock private JdbcClientPool mockClientPool; + @Mock private QueryRunner mockQueryRunner; + @Mock private ResultSet mockResultSet; + + private JdbcSnowflakeClient snowflakeClient; + + @Before + public void before() throws SQLException, InterruptedException { + snowflakeClient = new JdbcSnowflakeClient(mockClientPool); + snowflakeClient.setQueryRunner(mockQueryRunner); + + doAnswer( + new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + return ((ClientPool.Action) invocation.getArguments()[0]).run(mockConnection); + } + }) + .when(mockClientPool) + .run(any(ClientPool.Action.class)); + doAnswer( + new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + return ((ResultSetHandler) invocation.getArguments()[2]).handle(mockResultSet); + } + }) + .when(mockQueryRunner) + .query( + any(Connection.class), + any(String.class), + any(ResultSetHandler.class), + ArgumentMatchers.any()); + } + + @Test + public void testListSchemasInAccount() throws SQLException { + when(mockResultSet.next()).thenReturn(true).thenReturn(true).thenReturn(true).thenReturn(false); + when(mockResultSet.getString("database_name")) + .thenReturn("DB_1") + .thenReturn("DB_1") + .thenReturn("DB_2"); + when(mockResultSet.getString("name")) + .thenReturn("SCHEMA_1") + .thenReturn("SCHEMA_2") + .thenReturn("SCHEMA_3"); + + List actualList = snowflakeClient.listSchemas(Namespace.of()); + + verify(mockQueryRunner) + .query( + eq(mockConnection), + eq("SHOW SCHEMAS IN ACCOUNT"), + any(ResultSetHandler.class), + eq((Object[]) null)); + + List expectedList = + Lists.newArrayList( + new SnowflakeSchema("DB_1", "SCHEMA_1"), + new SnowflakeSchema("DB_1", "SCHEMA_2"), + new SnowflakeSchema("DB_2", "SCHEMA_3")); + Assertions.assertThat(actualList).hasSameElementsAs(expectedList); + } + + @Test + public void testListSchemasInDatabase() throws SQLException { + when(mockResultSet.next()).thenReturn(true).thenReturn(true).thenReturn(false); + when(mockResultSet.getString("database_name")).thenReturn("DB_1").thenReturn("DB_1"); + when(mockResultSet.getString("name")).thenReturn("SCHEMA_1").thenReturn("SCHEMA_2"); + + List actualList = snowflakeClient.listSchemas(Namespace.of("DB_1")); + + verify(mockQueryRunner) + .query( + eq(mockConnection), + eq("SHOW SCHEMAS IN DATABASE IDENTIFIER(?)"), + any(ResultSetHandler.class), + eq("DB_1")); + + List expectedList = + Lists.newArrayList( + new SnowflakeSchema("DB_1", "SCHEMA_1"), new SnowflakeSchema("DB_1", "SCHEMA_2")); + Assertions.assertThat(actualList).hasSameElementsAs(expectedList); + } + + @Test + public void testListSchemasSQLException() throws SQLException, InterruptedException { + when(mockClientPool.run(any(ClientPool.Action.class))) + .thenThrow(new SQLException("Fake SQL exception")); + Assert.assertThrows( + UncheckedSQLException.class, () -> snowflakeClient.listSchemas(Namespace.of("DB_1"))); + } + + @Test + public void testListSchemasInterruptedException() throws SQLException, InterruptedException { + when(mockClientPool.run(any(ClientPool.Action.class))) + .thenThrow(new InterruptedException("Fake interrupted exception")); + Assert.assertThrows( + UncheckedInterruptedException.class, + () -> snowflakeClient.listSchemas(Namespace.of("DB_1"))); + } + + @Test + public void testListIcebergTablesInAccount() throws SQLException { + when(mockResultSet.next()) + .thenReturn(true) + .thenReturn(true) + .thenReturn(true) + .thenReturn(true) + .thenReturn(false); + when(mockResultSet.getString("database_name")) + .thenReturn("DB_1") + .thenReturn("DB_1") + .thenReturn("DB_1") + .thenReturn("DB_2"); + when(mockResultSet.getString("schema_name")) + .thenReturn("SCHEMA_1") + .thenReturn("SCHEMA_1") + .thenReturn("SCHEMA_2") + .thenReturn("SCHEMA_3"); + when(mockResultSet.getString("name")) + .thenReturn("TABLE_1") + .thenReturn("TABLE_2") + .thenReturn("TABLE_3") + .thenReturn("TABLE_4"); + + List actualList = snowflakeClient.listIcebergTables(Namespace.of()); + + verify(mockQueryRunner) + .query( + eq(mockConnection), + eq("SHOW ICEBERG TABLES IN ACCOUNT"), + any(ResultSetHandler.class), + eq((Object[]) null)); + + List expectedList = + Lists.newArrayList( + new SnowflakeTable("DB_1", "SCHEMA_1", "TABLE_1"), + new SnowflakeTable("DB_1", "SCHEMA_1", "TABLE_2"), + new SnowflakeTable("DB_1", "SCHEMA_2", "TABLE_3"), + new SnowflakeTable("DB_2", "SCHEMA_3", "TABLE_4")); + Assertions.assertThat(actualList).hasSameElementsAs(expectedList); + } + + @Test + public void testListIcebergTablesInDatabase() throws SQLException { + when(mockResultSet.next()).thenReturn(true).thenReturn(true).thenReturn(true).thenReturn(false); + when(mockResultSet.getString("database_name")) + .thenReturn("DB_1") + .thenReturn("DB_1") + .thenReturn("DB_1"); + when(mockResultSet.getString("schema_name")) + .thenReturn("SCHEMA_1") + .thenReturn("SCHEMA_1") + .thenReturn("SCHEMA_2"); + when(mockResultSet.getString("name")) + .thenReturn("TABLE_1") + .thenReturn("TABLE_2") + .thenReturn("TABLE_3"); + + List actualList = snowflakeClient.listIcebergTables(Namespace.of("DB_1")); + + verify(mockQueryRunner) + .query( + eq(mockConnection), + eq("SHOW ICEBERG TABLES IN DATABASE IDENTIFIER(?)"), + any(ResultSetHandler.class), + eq("DB_1")); + + List expectedList = + Lists.newArrayList( + new SnowflakeTable("DB_1", "SCHEMA_1", "TABLE_1"), + new SnowflakeTable("DB_1", "SCHEMA_1", "TABLE_2"), + new SnowflakeTable("DB_1", "SCHEMA_2", "TABLE_3")); + Assertions.assertThat(actualList).hasSameElementsAs(expectedList); + } + + @Test + public void testListIcebergTablesInSchema() throws SQLException { + when(mockResultSet.next()).thenReturn(true).thenReturn(true).thenReturn(false); + when(mockResultSet.getString("database_name")).thenReturn("DB_1").thenReturn("DB_1"); + when(mockResultSet.getString("schema_name")).thenReturn("SCHEMA_1").thenReturn("SCHEMA_1"); + when(mockResultSet.getString("name")).thenReturn("TABLE_1").thenReturn("TABLE_2"); + + List actualList = + snowflakeClient.listIcebergTables(Namespace.of("DB_1", "SCHEMA_1")); + + verify(mockQueryRunner) + .query( + eq(mockConnection), + eq("SHOW ICEBERG TABLES IN SCHEMA IDENTIFIER(?)"), + any(ResultSetHandler.class), + eq("DB_1.SCHEMA_1")); + + List expectedList = + Lists.newArrayList( + new SnowflakeTable("DB_1", "SCHEMA_1", "TABLE_1"), + new SnowflakeTable("DB_1", "SCHEMA_1", "TABLE_2")); + Assertions.assertThat(actualList).hasSameElementsAs(expectedList); + } + + @Test + public void testListIcebergTablesSQLException() throws SQLException, InterruptedException { + when(mockClientPool.run(any(ClientPool.Action.class))) + .thenThrow(new SQLException("Fake SQL exception")); + Assert.assertThrows( + UncheckedSQLException.class, () -> snowflakeClient.listIcebergTables(Namespace.of("DB_1"))); + } + + @Test + public void testListIcebergTablesInterruptedException() + throws SQLException, InterruptedException { + when(mockClientPool.run(any(ClientPool.Action.class))) + .thenThrow(new InterruptedException("Fake interrupted exception")); + Assert.assertThrows( + UncheckedInterruptedException.class, + () -> snowflakeClient.listIcebergTables(Namespace.of("DB_1"))); + } + + @Test + public void testGetS3TableMetadata() throws SQLException { + when(mockResultSet.next()).thenReturn(true); + when(mockResultSet.getString("METADATA")) + .thenReturn( + "{\"metadataLocation\":\"s3://tab1/metadata/v3.metadata.json\",\"status\":\"success\"}"); + + SnowflakeTableMetadata actualMetadata = + snowflakeClient.getTableMetadata( + TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1")); + + verify(mockQueryRunner) + .query( + eq(mockConnection), + eq("SELECT SYSTEM$GET_ICEBERG_TABLE_INFORMATION(?) AS METADATA"), + any(ResultSetHandler.class), + eq("DB_1.SCHEMA_1.TABLE_1")); + + SnowflakeTableMetadata expectedMetadata = + new SnowflakeTableMetadata( + "s3://tab1/metadata/v3.metadata.json", + "s3://tab1/metadata/v3.metadata.json", + "success", + null); + Assert.assertEquals(expectedMetadata, actualMetadata); + } + + @Test + public void testGetAzureTableMetadata() throws SQLException { + when(mockResultSet.next()).thenReturn(true); + when(mockResultSet.getString("METADATA")) + .thenReturn( + "{\"metadataLocation\":\"azure://myaccount.blob.core.windows.net/mycontainer/tab3/metadata/v334.metadata.json\",\"status\":\"success\"}"); + + SnowflakeTableMetadata actualMetadata = + snowflakeClient.getTableMetadata( + TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1")); + + verify(mockQueryRunner) + .query( + eq(mockConnection), + eq("SELECT SYSTEM$GET_ICEBERG_TABLE_INFORMATION(?) AS METADATA"), + any(ResultSetHandler.class), + eq("DB_1.SCHEMA_1.TABLE_1")); + + SnowflakeTableMetadata expectedMetadata = + new SnowflakeTableMetadata( + "azure://myaccount.blob.core.windows.net/mycontainer/tab3/metadata/v334.metadata.json", + "wasbs://mycontainer@myaccount.blob.core.windows.net/tab3/metadata/v334.metadata.json", + "success", + null); + Assert.assertEquals(expectedMetadata, actualMetadata); + } + + @Test + public void testGetGcsTableMetadata() throws SQLException { + when(mockResultSet.next()).thenReturn(true); + when(mockResultSet.getString("METADATA")) + .thenReturn( + "{\"metadataLocation\":\"gcs://tab5/metadata/v793.metadata.json\",\"status\":\"success\"}"); + + SnowflakeTableMetadata actualMetadata = + snowflakeClient.getTableMetadata( + TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1")); + + verify(mockQueryRunner) + .query( + eq(mockConnection), + eq("SELECT SYSTEM$GET_ICEBERG_TABLE_INFORMATION(?) AS METADATA"), + any(ResultSetHandler.class), + eq("DB_1.SCHEMA_1.TABLE_1")); + + SnowflakeTableMetadata expectedMetadata = + new SnowflakeTableMetadata( + "gcs://tab5/metadata/v793.metadata.json", + "gs://tab5/metadata/v793.metadata.json", + "success", + null); + Assert.assertEquals(expectedMetadata, actualMetadata); + } + + @Test + public void testGetTableMetadataMalformedJson() throws SQLException { + when(mockResultSet.next()).thenReturn(true); + when(mockResultSet.getString("METADATA")).thenReturn("{\"malformed_no_closing_bracket"); + Assert.assertThrows( + IllegalArgumentException.class, + () -> + snowflakeClient.getTableMetadata( + TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1"))); + } + + @Test + public void testGetTableMetadataSQLException() throws SQLException, InterruptedException { + when(mockClientPool.run(any(ClientPool.Action.class))) + .thenThrow(new SQLException("Fake SQL exception")); + Assert.assertThrows( + UncheckedSQLException.class, + () -> + snowflakeClient.getTableMetadata( + TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1"))); + } + + @Test + public void testGetTableMetadataInterruptedException() throws SQLException, InterruptedException { + when(mockClientPool.run(any(ClientPool.Action.class))) + .thenThrow(new InterruptedException("Fake interrupted exception")); + Assert.assertThrows( + UncheckedInterruptedException.class, + () -> + snowflakeClient.getTableMetadata( + TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1"))); + } + + @Test + public void testClose() { + snowflakeClient.close(); + verify(mockClientPool).close(); + } +} From 94d2e622515e0a0cfbabf9b3574d56e6558d7c05 Mon Sep 17 00:00:00 2001 From: Dennis Huo Date: Wed, 14 Dec 2022 14:29:22 -0800 Subject: [PATCH 2/2] Add comments for each test case, update target Spark runtime versions to be included. --- .../snowflake/JdbcSnowflakeClientTest.java | 58 +++++++++++++++++++ spark/v3.1/build.gradle | 3 + spark/v3.2/build.gradle | 3 + 3 files changed, 64 insertions(+) diff --git a/snowflake/src/test/java/org/apache/iceberg/snowflake/JdbcSnowflakeClientTest.java b/snowflake/src/test/java/org/apache/iceberg/snowflake/JdbcSnowflakeClientTest.java index 2757dbdf5989..0e5b881b1c29 100644 --- a/snowflake/src/test/java/org/apache/iceberg/snowflake/JdbcSnowflakeClientTest.java +++ b/snowflake/src/test/java/org/apache/iceberg/snowflake/JdbcSnowflakeClientTest.java @@ -89,6 +89,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { ArgumentMatchers.any()); } + /** + * For the root/empty Namespace, expect an underlying query to list schemas at the ACCOUNT level + * with no query parameters. + */ @Test public void testListSchemasInAccount() throws SQLException { when(mockResultSet.next()).thenReturn(true).thenReturn(true).thenReturn(true).thenReturn(false); @@ -118,6 +122,10 @@ public void testListSchemasInAccount() throws SQLException { Assertions.assertThat(actualList).hasSameElementsAs(expectedList); } + /** + * For a 1-level Namespace, expect an underlying query to list schemas at the DATABASE level and + * supply the Namespace as a query param in an IDENTIFIER. + */ @Test public void testListSchemasInDatabase() throws SQLException { when(mockResultSet.next()).thenReturn(true).thenReturn(true).thenReturn(false); @@ -139,6 +147,10 @@ public void testListSchemasInDatabase() throws SQLException { Assertions.assertThat(actualList).hasSameElementsAs(expectedList); } + /** + * Any unexpected SQLException from the underlying connection will propagate out as an + * UncheckedSQLException when listing schemas. + */ @Test public void testListSchemasSQLException() throws SQLException, InterruptedException { when(mockClientPool.run(any(ClientPool.Action.class))) @@ -147,6 +159,10 @@ public void testListSchemasSQLException() throws SQLException, InterruptedExcept UncheckedSQLException.class, () -> snowflakeClient.listSchemas(Namespace.of("DB_1"))); } + /** + * Any unexpected InterruptedException from the underlying connection will propagate out as an + * UncheckedInterruptedException when listing schemas. + */ @Test public void testListSchemasInterruptedException() throws SQLException, InterruptedException { when(mockClientPool.run(any(ClientPool.Action.class))) @@ -156,6 +172,10 @@ public void testListSchemasInterruptedException() throws SQLException, Interrupt () -> snowflakeClient.listSchemas(Namespace.of("DB_1"))); } + /** + * For the root/empty Namespace, expect an underlying query to list tables at the ACCOUNT level + * with no query parameters. + */ @Test public void testListIcebergTablesInAccount() throws SQLException { when(mockResultSet.next()) @@ -198,6 +218,10 @@ public void testListIcebergTablesInAccount() throws SQLException { Assertions.assertThat(actualList).hasSameElementsAs(expectedList); } + /** + * For a 1-level Namespace, expect an underlying query to list tables at the DATABASE level and + * supply the Namespace as a query param in an IDENTIFIER. + */ @Test public void testListIcebergTablesInDatabase() throws SQLException { when(mockResultSet.next()).thenReturn(true).thenReturn(true).thenReturn(true).thenReturn(false); @@ -231,6 +255,10 @@ public void testListIcebergTablesInDatabase() throws SQLException { Assertions.assertThat(actualList).hasSameElementsAs(expectedList); } + /** + * For a 2-level Namespace, expect an underlying query to list tables at the SCHEMA level and + * supply the Namespace as a query param in an IDENTIFIER. + */ @Test public void testListIcebergTablesInSchema() throws SQLException { when(mockResultSet.next()).thenReturn(true).thenReturn(true).thenReturn(false); @@ -255,6 +283,10 @@ public void testListIcebergTablesInSchema() throws SQLException { Assertions.assertThat(actualList).hasSameElementsAs(expectedList); } + /** + * Any unexpected SQLException from the underlying connection will propagate out as an + * UncheckedSQLException when listing tables. + */ @Test public void testListIcebergTablesSQLException() throws SQLException, InterruptedException { when(mockClientPool.run(any(ClientPool.Action.class))) @@ -263,6 +295,10 @@ public void testListIcebergTablesSQLException() throws SQLException, Interrupted UncheckedSQLException.class, () -> snowflakeClient.listIcebergTables(Namespace.of("DB_1"))); } + /** + * Any unexpected InterruptedException from the underlying connection will propagate out as an + * UncheckedInterruptedException when listing tables. + */ @Test public void testListIcebergTablesInterruptedException() throws SQLException, InterruptedException { @@ -273,6 +309,10 @@ public void testListIcebergTablesInterruptedException() () -> snowflakeClient.listIcebergTables(Namespace.of("DB_1"))); } + /** + * Test parsing of table metadata JSON from a GET_ICEBERG_TABLE_INFORMATION call, with the S3 path + * unaltered between snowflake/iceberg path representations. + */ @Test public void testGetS3TableMetadata() throws SQLException { when(mockResultSet.next()).thenReturn(true); @@ -300,6 +340,10 @@ public void testGetS3TableMetadata() throws SQLException { Assert.assertEquals(expectedMetadata, actualMetadata); } + /** + * Test parsing of table metadata JSON from a GET_ICEBERG_TABLE_INFORMATION call, with the Azure + * path translated from an azure:// format to a wasbs:// format. + */ @Test public void testGetAzureTableMetadata() throws SQLException { when(mockResultSet.next()).thenReturn(true); @@ -327,6 +371,10 @@ public void testGetAzureTableMetadata() throws SQLException { Assert.assertEquals(expectedMetadata, actualMetadata); } + /** + * Test parsing of table metadata JSON from a GET_ICEBERG_TABLE_INFORMATION call, with the GCS + * path translated from a gcs:// format to a gs:// format. + */ @Test public void testGetGcsTableMetadata() throws SQLException { when(mockResultSet.next()).thenReturn(true); @@ -354,6 +402,7 @@ public void testGetGcsTableMetadata() throws SQLException { Assert.assertEquals(expectedMetadata, actualMetadata); } + /** Malformed JSON from a ResultSet should propagate as an IllegalArgumentException. */ @Test public void testGetTableMetadataMalformedJson() throws SQLException { when(mockResultSet.next()).thenReturn(true); @@ -365,6 +414,10 @@ public void testGetTableMetadataMalformedJson() throws SQLException { TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1"))); } + /** + * Any unexpected SQLException from the underlying connection will propagate out as an + * UncheckedSQLException when getting table metadata. + */ @Test public void testGetTableMetadataSQLException() throws SQLException, InterruptedException { when(mockClientPool.run(any(ClientPool.Action.class))) @@ -376,6 +429,10 @@ public void testGetTableMetadataSQLException() throws SQLException, InterruptedE TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1"))); } + /** + * Any unexpected InterruptedException from the underlying connection will propagate out as an + * UncheckedInterruptedException when getting table metadata. + */ @Test public void testGetTableMetadataInterruptedException() throws SQLException, InterruptedException { when(mockClientPool.run(any(ClientPool.Action.class))) @@ -387,6 +444,7 @@ public void testGetTableMetadataInterruptedException() throws SQLException, Inte TableIdentifier.of(Namespace.of("DB_1", "SCHEMA_1"), "TABLE_1"))); } + /** Calling close() propagates to closing underlying client pool. */ @Test public void testClose() { snowflakeClient.close(); diff --git a/spark/v3.1/build.gradle b/spark/v3.1/build.gradle index eca34afcbd02..c7861d36e555 100644 --- a/spark/v3.1/build.gradle +++ b/spark/v3.1/build.gradle @@ -213,6 +213,9 @@ project(':iceberg-spark:iceberg-spark-runtime-3.1_2.12') { implementation(project(':iceberg-nessie')) { exclude group: 'com.google.code.findbugs', module: 'jsr305' } + implementation (project(':iceberg-snowflake')) { + exclude group: 'net.snowflake' , module: 'snowflake-jdbc' + } integrationImplementation "org.apache.spark:spark-hive_2.12:${sparkVersion}" integrationImplementation 'org.junit.vintage:junit-vintage-engine' diff --git a/spark/v3.2/build.gradle b/spark/v3.2/build.gradle index 8de93d0df8ac..971decde100e 100644 --- a/spark/v3.2/build.gradle +++ b/spark/v3.2/build.gradle @@ -222,6 +222,9 @@ project(":iceberg-spark:iceberg-spark-runtime-${sparkMajorVersion}_${scalaVersio implementation(project(':iceberg-nessie')) { exclude group: 'com.google.code.findbugs', module: 'jsr305' } + implementation (project(':iceberg-snowflake')) { + exclude group: 'net.snowflake' , module: 'snowflake-jdbc' + } integrationImplementation "org.scala-lang.modules:scala-collection-compat_${scalaVersion}" integrationImplementation "org.apache.spark:spark-hive_${scalaVersion}:${sparkVersion}"