You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "manisin (via GitHub)" <gi...@apache.org> on 2023/03/03 22:04:21 UTC
[GitHub] [iceberg] manisin opened a new pull request, #7004: Integration tests for Snowflake catalog
manisin opened a new pull request, #7004:
URL: https://github.com/apache/iceberg/pull/7004
This PR adds integration tests to the Snowflake catalog. The initial set of tests covers only the namespace related scenarios like listing, loading, handling special characters etc.
The current set of integration tests covers features that are not iceberg specific so they are available outside of the private preview scope.
The test suite would need following environment variables to be set:
SNOW_URI: [Required] JDBC driver URI, e.g. jdbc:snowflake://https://account.snowflakecomputing.com/
SNOW_TEST_DB_NAME: [Required] Test database to create, e.g. Test_db
SNOW_USER: [Optional] JDBC user, could be specified as part of uri as well, e.g. admin
SNOW_PASSWORD: [Optional] JDBC password, could be specified as part of uri as well, e.g. password
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1130235554
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
Review Comment:
There's no metadata associated with namespaces in snowflake yet so just verifying the no throw for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1133534241
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,253 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA")));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema = "Schema_1";
+ String dbName = getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema)))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("listNamespaces must be at either ROOT or DATABASE level");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb)))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema)))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = getDatabase();
+ Assertions.assertThatThrownBy(
+ () ->
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel")))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Snowflake max namespace level is");
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"Schema", "schema", "_schema", "Schema123", "schema$", "_Schema$_123"})
+ public void testLoadNamespaceWithValidUnquotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)))
+ .isEmpty();
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema = "Schema123";
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)))
+ .isEmpty();
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema.toUpperCase())))
+ .isEmpty();
+ Assertions.assertThat(
Review Comment:
what about using `@ValueSource` for all of those?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1126050242
##########
build.gradle:
##########
@@ -834,6 +839,21 @@ project(':iceberg-snowflake') {
runtimeOnly("net.snowflake:snowflake-jdbc")
testImplementation project(path: ':iceberg-core', configuration: 'testArtifacts')
+ integrationImplementation("net.snowflake:snowflake-jdbc")
+ }
+
+ sourceSets {
+ integration {
+ java.srcDir "$projectDir/src/integration/java"
+ resources.srcDir "$projectDir/src/integration/resources"
+ compileClasspath += main.output + test.output
+ runtimeClasspath += main.output + test.output
+ }
+ }
+
+ task integrationTest(type: Test) {
+ testClassesDirs = sourceSets.integration.output.classesDirs
+ classpath = sourceSets.integration.runtimeClasspath
Review Comment:
this also requires `check.dependsOn integrationTest` below L857, otherwise integration tests are never running as part of CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1480794398
@danielcweeks could you review this one please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Integration tests for Snowflake catalog [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-2323082556
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1127070405
##########
build.gradle:
##########
@@ -834,6 +839,21 @@ project(':iceberg-snowflake') {
runtimeOnly("net.snowflake:snowflake-jdbc")
testImplementation project(path: ':iceberg-core', configuration: 'testArtifacts')
+ integrationImplementation("net.snowflake:snowflake-jdbc")
+ }
+
+ sourceSets {
+ integration {
+ java.srcDir "$projectDir/src/integration/java"
+ resources.srcDir "$projectDir/src/integration/resources"
+ compileClasspath += main.output + test.output
+ runtimeClasspath += main.output + test.output
+ }
+ }
+
+ task integrationTest(type: Test) {
+ testClassesDirs = sourceSets.integration.output.classesDirs
+ classpath = sourceSets.integration.runtimeClasspath
Review Comment:
The decision to not run integrationTest as part of CI is intentional until we reach milestone where snowflake side APIs/Syntax are documented and stable. In current state, the integration tests are meant to be run by the catalog developers who have access to Private preview.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1130234704
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/SnowTestBase.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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 java.sql.PreparedStatement;
+import java.sql.SQLException;
+import org.apache.iceberg.jdbc.JdbcClientPool;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.junit.BeforeClass;
+import org.junit.jupiter.api.AfterAll;
Review Comment:
Moved to Junit5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1127693987
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
Review Comment:
I believe the additional `new ArrayList<Namespace>(..)` is not required
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
Review Comment:
rather than using `Assertions.assertThatNoException()` it would be better to assert that the correct metadata is returned
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/SnowTestBase.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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 java.sql.PreparedStatement;
+import java.sql.SQLException;
+import org.apache.iceberg.jdbc.JdbcClientPool;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.junit.BeforeClass;
+import org.junit.jupiter.api.AfterAll;
+
+@SuppressWarnings("VisibilityModifier")
+class SnowTestBase {
+
+ static SnowflakeCatalog snowflakeCatalog;
+
+ static JdbcClientPool clientPool;
+
+ protected SnowTestBase() {}
+
+ @BeforeClass
+ public static void beforeAll() {
+ snowflakeCatalog = new SnowflakeCatalog();
+ TestConfigurations configs = TestConfigurations.getInstance();
Review Comment:
Why not just expose the respective fields (db, user, uri) in `SnowTestBase` for subclasses rather than introducing `TestConfigurations`?
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1.toUpperCase()));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, "schEmA123"));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierWithLeadingUnderscoreAndDollarCharacters()
+ throws SQLException, InterruptedException {
+ String schema1 = "_Schema$_123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
Review Comment:
reading through all the test methods around this, I think it would make sense to use [JUnit5 + parameterized tests](https://junit.org/junit5/docs/current/user-guide/#writing-tests-parameterized-tests) because the tests mainly differ in the schema name
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
Review Comment:
as mentioned on `SnowTestBase` it would be better to switch to Junit5 tests for new stuff
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1.toUpperCase()));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, "schEmA123"));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierWithLeadingUnderscoreAndDollarCharacters()
+ throws SQLException, InterruptedException {
+ String schema1 = "_Schema$_123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
Review Comment:
same as above
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
Review Comment:
all of these checks that verify a certain type of exception is thrown should have a check for the error message: `hasMessage(...)`
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
Review Comment:
same as above, it's better to verify that the correct namespace metadata is returned
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1.toUpperCase()));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, "schEmA123"));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierWithLeadingUnderscoreAndDollarCharacters()
+ throws SQLException, InterruptedException {
+ String schema1 = "_Schema$_123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithQuotedIdentifierIsCaseSensitive()
+ throws SQLException, InterruptedException {
+ String schema1 = "\"Schema123\"";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1.toUpperCase()));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithQuotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "\"Schema123\"";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithQuotedIdentifierWithSpecialCharacters()
+ throws SQLException, InterruptedException {
+ String schema1 = "\"H@!!.0-w*r()^'\"";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testDropNamespaceNotSupported() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.dropNamespace(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(UnsupportedOperationException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testSetPropertiesNotSupported() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.setProperties(
+ Namespace.of(dbName, schema1), new HashMap<String, String>());
+ })
+ .isInstanceOf(UnsupportedOperationException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testRemovePropertiesNotSupported() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatException()
+ .isThrownBy(
Review Comment:
`.isThrownBy(..)` should only contain the line of code that actually throws the exception. Same for all the other test methods in this class
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1130642011
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,268 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA")));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> {
Review Comment:
nit: no need for the {} here and all the places below
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,268 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA")));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema));
+ })
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("listNamespaces must be at either ROOT or DATABASE level");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
Review Comment:
it is generally good pratice to assert for a specific error message, so would be good to have `.hasMessageContaining` here and all the other places below
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,268 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA")));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema));
+ })
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("listNamespaces must be at either ROOT or DATABASE level");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"Schema", "schema", "_schema", "Schema123", "schema$", "_Schema$_123"})
Review Comment:
looks much better with this now :100:
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,268 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA")));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema));
+ })
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("listNamespaces must be at either ROOT or DATABASE level");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"Schema", "schema", "_schema", "Schema123", "schema$", "_Schema$_123"})
+ public void testLoadNamespaceWithValidUnquotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema));
+ });
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema.toUpperCase()));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, "schEmA123"));
+ });
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithQuotedIdentifierIsCaseSensitive()
+ throws SQLException, InterruptedException {
+ String schema = "\"Schema123\"";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema.toUpperCase()));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "\"Schema123\"",
+ "\"MySchema\"",
+ "\"my.schema\"",
+ "\"my schema\"",
+ "\"My 'Schema'\"",
+ "\"3rd_schema\"",
+ "\"$Ischema\"",
+ "\"H@!!.0-w*r()^'\"",
+ "\"идентификатор\""
+ })
+ public void testLoadNamespaceWithQuotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema));
+ });
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testDropNamespaceNotSupported() throws SQLException, InterruptedException {
+ String schema = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.dropNamespace(Namespace.of(dbName, schema));
+ })
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessageContaining("SnowflakeCatalog does not currently support dropNamespace");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testSetPropertiesNotSupported() throws SQLException, InterruptedException {
+ String schema = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.setProperties(
+ Namespace.of(dbName, schema), new HashMap<String, String>());
+ })
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessageContaining("SnowflakeCatalog does not currently support setProperties");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testRemovePropertiesNotSupported() throws SQLException, InterruptedException {
+ String schema = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.removeProperties(
+ Namespace.of(dbName, schema), new HashSet<String>());
Review Comment:
nit (I think checkstyle should even complain about this): `Sets.newHashSet()` or `ImmutableSet.of()`
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/SnowTestBase.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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 java.sql.PreparedStatement;
+import java.sql.SQLException;
+import org.apache.iceberg.jdbc.JdbcClientPool;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.junit.BeforeClass;
+import org.junit.jupiter.api.AfterAll;
+
+@SuppressWarnings("VisibilityModifier")
+class SnowTestBase {
+
+ static SnowflakeCatalog snowflakeCatalog;
+
+ static JdbcClientPool clientPool;
+
+ protected SnowTestBase() {}
+
+ @BeforeClass
+ public static void beforeAll() {
+ snowflakeCatalog = new SnowflakeCatalog();
+ TestConfigurations configs = TestConfigurations.getInstance();
Review Comment:
I believe pretty much all other tests across the codebase just set things up in a base test class (like the one you have here). I think moving things from `TestConfigurations` to this class makes the code cleaner, but I'm curious what other reviewers think
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1489513438
> @danielcweeks could you review this one please?
@danielcweeks , gentle ping to review this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1455645231
@manisin just a general question, but is there some sort of Docker image(s) that could be used for integration testing this with [testcontainers](https://www.testcontainers.org/)?
I'm curious what the general approach for integration testing is for this. The PR as it stands right now doesn't run as part of normal CI (as I commented above) and there's also no separate yml file that would set this up for its own CI runs (with the env vars that are mentioned in the README).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1130235952
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1.toUpperCase()));
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, "schEmA123"));
+ });
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierWithLeadingUnderscoreAndDollarCharacters()
+ throws SQLException, InterruptedException {
+ String schema1 = "_Schema$_123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
Review Comment:
Switched to parameterized tests and it simplified tests. Thanks for the suggestion!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1461093196
> @manisin just a general question, but is there some sort of Docker image(s) that could be used for integration testing this with [testcontainers](https://www.testcontainers.org/)? I'm curious what the general approach for integration testing is for this. The PR as it stands right now doesn't run as part of normal CI (as I commented above) and there's also no separate yml file that would set this up for its own CI runs (with the env vars that are mentioned in the README).
@nastra there are no images/test containers to deploy and test snowflake. The approach we are considering it to add a separate yml file ( like this https://github.com/Snowflake-Labs/iceberg/blob/1f0dec004f83e1000f65cb552c360dc28eaf7559/.github/workflows/snowflake-ci.yml) and provision a snowflake deployment (with secrets onboarded to github) for integration testing with iceberg. Let me know your thoughts and suggestions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1127687291
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/SnowTestBase.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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 java.sql.PreparedStatement;
+import java.sql.SQLException;
+import org.apache.iceberg.jdbc.JdbcClientPool;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.junit.BeforeClass;
+import org.junit.jupiter.api.AfterAll;
Review Comment:
it looks like this uses a mix of Junit 4 and 5, would be better to switch completely to Junit5 for new tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1131450701
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/SnowTestBase.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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 java.sql.PreparedStatement;
+import java.sql.SQLException;
+import org.apache.iceberg.jdbc.JdbcClientPool;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.junit.BeforeClass;
+import org.junit.jupiter.api.AfterAll;
+
+@SuppressWarnings("VisibilityModifier")
+class SnowTestBase {
+
+ static SnowflakeCatalog snowflakeCatalog;
+
+ static JdbcClientPool clientPool;
+
+ protected SnowTestBase() {}
+
+ @BeforeClass
+ public static void beforeAll() {
+ snowflakeCatalog = new SnowflakeCatalog();
+ TestConfigurations configs = TestConfigurations.getInstance();
Review Comment:
Moved configs to base class to keep the pattern uniform across tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1130237674
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/SnowTestBase.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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 java.sql.PreparedStatement;
+import java.sql.SQLException;
+import org.apache.iceberg.jdbc.JdbcClientPool;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.junit.BeforeClass;
+import org.junit.jupiter.api.AfterAll;
+
+@SuppressWarnings("VisibilityModifier")
+class SnowTestBase {
+
+ static SnowflakeCatalog snowflakeCatalog;
+
+ static JdbcClientPool clientPool;
+
+ protected SnowTestBase() {}
+
+ @BeforeClass
+ public static void beforeAll() {
+ snowflakeCatalog = new SnowflakeCatalog();
+ TestConfigurations configs = TestConfigurations.getInstance();
Review Comment:
Favoring the separation of configurations from common utility functions in the base class for ease of readability and separation of concerns. I can move them to base class if you feel strongly about this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Integration tests for Snowflake catalog [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-2308585136
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Integration tests for Snowflake catalog [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #7004: Integration tests for Snowflake catalog
URL: https://github.com/apache/iceberg/pull/7004
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1495101199
@nastra we have few PRs that are pending in review for almost a month now. Could you help getting some traction for merging/reviewing process?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1131461866
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,258 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA")));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema = "Schema_1";
+ String dbName = getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema)))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("listNamespaces must be at either ROOT or DATABASE level");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb)))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema)))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = getDatabase();
+ Assertions.assertThatThrownBy(
+ () ->
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel")))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Snowflake max namespace level is");
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"Schema", "schema", "_schema", "Schema123", "schema$", "_Schema$_123"})
+ public void testLoadNamespaceWithValidUnquotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema = "Schema123";
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
+ Assertions.assertThat(
+ snowflakeCatalog
+ .loadNamespaceMetadata(Namespace.of(dbName, schema.toUpperCase()))
+ .isEmpty())
+ .isTrue();
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, "schEmA123")).isEmpty())
+ .isTrue();
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithQuotedIdentifierIsCaseSensitive()
+ throws SQLException, InterruptedException {
+ String schema = "\"Schema123\"";
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () ->
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(dbName, schema.toUpperCase())))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "\"Schema123\"",
+ "\"MySchema\"",
+ "\"my.schema\"",
+ "\"my schema\"",
+ "\"My 'Schema'\"",
+ "\"3rd_schema\"",
+ "\"$Ischema\"",
+ "\"H@!!.0-w*r()^'\"",
+ "\"идентификатор\""
+ })
+ public void testLoadNamespaceWithQuotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
Review Comment:
it should be `Assertions.assertThat(snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema))).isEmpty()` where the `isEmpty()` call is on the outer `assertThat(..)` -> `assertThat(..).isEmpty()`.
The big advantage of that is that it will actually print the content of the map if the assertion ever fails, whereas `isTrue()` will only tell you that it's not true
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1131516814
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,258 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA")));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema = "Schema_1";
+ String dbName = getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema)))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("listNamespaces must be at either ROOT or DATABASE level");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb)))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema)))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = getDatabase();
+ Assertions.assertThatThrownBy(
+ () ->
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel")))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Snowflake max namespace level is");
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"Schema", "schema", "_schema", "Schema123", "schema$", "_Schema$_123"})
+ public void testLoadNamespaceWithValidUnquotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema = "Schema123";
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
+ Assertions.assertThat(
+ snowflakeCatalog
+ .loadNamespaceMetadata(Namespace.of(dbName, schema.toUpperCase()))
+ .isEmpty())
+ .isTrue();
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, "schEmA123")).isEmpty())
+ .isTrue();
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithQuotedIdentifierIsCaseSensitive()
+ throws SQLException, InterruptedException {
+ String schema = "\"Schema123\"";
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () ->
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(dbName, schema.toUpperCase())))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "\"Schema123\"",
+ "\"MySchema\"",
+ "\"my.schema\"",
+ "\"my schema\"",
+ "\"My 'Schema'\"",
+ "\"3rd_schema\"",
+ "\"$Ischema\"",
+ "\"H@!!.0-w*r()^'\"",
+ "\"идентификатор\""
+ })
+ public void testLoadNamespaceWithQuotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
Review Comment:
Thanks @nastra! Switched to the recommendation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1613172199
@manisin sorry that this is taking so long. For these integration tests I think it would be great to have a proper environment where those would automatically run. That means we'd need to add a GH action as part of this PR so that integration tests are executed with all the other CI runs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1130640521
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,290 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ new ArrayList<Namespace>(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA"))));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String dbName = TestConfigurations.getInstance().getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ Assertions.assertThatException()
+ .isThrownBy(
+ () -> {
+ snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema1));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ } finally {
+ dropSchemaIfExists(schema1);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema));
+ })
+ .isInstanceOf(NoSuchNamespaceException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = TestConfigurations.getInstance().getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> {
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel"));
+ })
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifier() throws SQLException, InterruptedException {
+ String schema1 = "Schema123";
+ String dbName = TestConfigurations.getInstance().getDatabase();
+ try {
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1));
Review Comment:
in that case we can just check that metadata is empty: `assertThat(snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema1))).isEmpty()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1464372309
> > > @manisin just a general question, but is there some sort of Docker image(s) that could be used for integration testing this with [testcontainers](https://www.testcontainers.org/)? I'm curious what the general approach for integration testing is for this. The PR as it stands right now doesn't run as part of normal CI (as I commented above) and there's also no separate yml file that would set this up for its own CI runs (with the env vars that are mentioned in the README).
> >
> >
> > @nastra there are no images/test containers to deploy and test snowflake. The approach we are considering it to add a separate yml file ( like this https://github.com/Snowflake-Labs/iceberg/blob/1f0dec004f83e1000f65cb552c360dc28eaf7559/.github/workflows/snowflake-ci.yml) and provision a snowflake deployment (with secrets onboarded to github) for integration testing with iceberg. Let me know your thoughts and suggestions.
>
> @manisin having a separate yml file for Snowflake integration tests should be fine, although I would probably include it in this PR.
@nastra if I include the yml file in this PR then the CI will fail as we need to onboard the secrets to github for the deployment provisioned on snowflake side. I can include the yml, if it's ok to start with a failing workflow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#discussion_r1131461866
##########
snowflake/src/integration/java/org/apache/iceberg/snowflake/NamespaceTests.java:
##########
@@ -0,0 +1,258 @@
+/*
+ * 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 java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class NamespaceTests extends SnowTestBase {
+
+ @Test
+ public void testListNamespacesAtRootLevel() throws SQLException, InterruptedException {
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.empty());
+ int dbCount =
+ clientPool.run(
+ conn -> {
+ int databaseCount = 0;
+ ResultSet rs = conn.createStatement().executeQuery("show databases");
+ while (rs.next()) {
+ databaseCount++;
+ }
+ return databaseCount;
+ });
+ Assertions.assertThat(namespaces.stream().count()).isEqualTo(dbCount);
+ }
+
+ @Test
+ public void testListNamespacesAtDatabaseLevel() throws SQLException, InterruptedException {
+ String schema1 = "Schema_1";
+ String schema2 = "Schema_2";
+ String dbName = getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema1);
+ createOrReplaceSchema(schema2);
+ List<Namespace> namespaces = snowflakeCatalog.listNamespaces(Namespace.of(dbName));
+ Assertions.assertThat(namespaces)
+ .containsExactlyInAnyOrderElementsOf(
+ Arrays.asList(
+ Namespace.of(dbName, schema1.toUpperCase()),
+ Namespace.of(dbName, schema2.toUpperCase()),
+ Namespace.of(dbName, "PUBLIC"),
+ Namespace.of(dbName, "INFORMATION_SCHEMA")));
+ } finally {
+ dropSchemaIfExists(schema1);
+ dropSchemaIfExists(schema2);
+ }
+ }
+
+ @Test
+ public void testListNamespacesAtSchemaLevelIsNotAllowed()
+ throws SQLException, InterruptedException {
+ String schema = "Schema_1";
+ String dbName = getDatabase().toUpperCase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.listNamespaces(Namespace.of(dbName, schema)))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("listNamespaces must be at either ROOT or DATABASE level");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNonExistingRootLevelNamespace() {
+ String nonExistingDb = "IDontExist";
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.loadNamespaceMetadata(Namespace.of(nonExistingDb)))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ }
+
+ @Test
+ public void testLoadNonExistingDBLevelNamespace() {
+ String nonExistingSchema = "IDontExist";
+ String db = getDatabase();
+ Assertions.assertThatThrownBy(
+ () -> snowflakeCatalog.loadNamespaceMetadata(Namespace.of(db, nonExistingSchema)))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ }
+
+ @Test
+ public void testLoadNamespaceThatExceedMaxSupportedHierarchy() {
+ String nonExistingSchema = "IDontExist";
+ String db = getDatabase();
+ Assertions.assertThatThrownBy(
+ () ->
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(db, nonExistingSchema, "UnSupportedLevel")))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Snowflake max namespace level is");
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"Schema", "schema", "_schema", "Schema123", "schema$", "_Schema$_123"})
+ public void testLoadNamespaceWithValidUnquotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithUnquotedIdentifierIsCaseInsensitive()
+ throws SQLException, InterruptedException {
+ String schema = "Schema123";
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
+ Assertions.assertThat(
+ snowflakeCatalog
+ .loadNamespaceMetadata(Namespace.of(dbName, schema.toUpperCase()))
+ .isEmpty())
+ .isTrue();
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, "schEmA123")).isEmpty())
+ .isTrue();
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @Test
+ public void testLoadNamespaceWithQuotedIdentifierIsCaseSensitive()
+ throws SQLException, InterruptedException {
+ String schema = "\"Schema123\"";
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThatThrownBy(
+ () ->
+ snowflakeCatalog.loadNamespaceMetadata(
+ Namespace.of(dbName, schema.toUpperCase())))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessageContainingAll("snowflake identifier", "doesn't exist");
+ } finally {
+ dropSchemaIfExists(schema);
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(
+ strings = {
+ "\"Schema123\"",
+ "\"MySchema\"",
+ "\"my.schema\"",
+ "\"my schema\"",
+ "\"My 'Schema'\"",
+ "\"3rd_schema\"",
+ "\"$Ischema\"",
+ "\"H@!!.0-w*r()^'\"",
+ "\"идентификатор\""
+ })
+ public void testLoadNamespaceWithQuotedIdentifier(String schema)
+ throws SQLException, InterruptedException {
+ String dbName = getDatabase();
+ try {
+ clientPool.run(conn -> conn.createStatement().execute("use " + dbName));
+ createOrReplaceSchema(schema);
+ Assertions.assertThat(
+ snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema)).isEmpty())
+ .isTrue();
Review Comment:
it should be `Assertions.assertThat(snowflakeCatalog.loadNamespaceMetadata(Namespace.of(dbName, schema))).isEmpty()` where the `isEmpty()` call is on the outer `assertThat(..)` -> `assertThat(..).isEmpty()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1461575601
> > @manisin just a general question, but is there some sort of Docker image(s) that could be used for integration testing this with [testcontainers](https://www.testcontainers.org/)? I'm curious what the general approach for integration testing is for this. The PR as it stands right now doesn't run as part of normal CI (as I commented above) and there's also no separate yml file that would set this up for its own CI runs (with the env vars that are mentioned in the README).
>
> @nastra there are no images/test containers to deploy and test snowflake. The approach we are considering it to add a separate yml file ( like this https://github.com/Snowflake-Labs/iceberg/blob/1f0dec004f83e1000f65cb552c360dc28eaf7559/.github/workflows/snowflake-ci.yml) and provision a snowflake deployment (with secrets onboarded to github) for integration testing with iceberg. Let me know your thoughts and suggestions.
@manisin having a separate yml file for Snowflake integration tests should be fine, although I would probably include it in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] manisin commented on pull request #7004: Integration tests for Snowflake catalog
Posted by "manisin (via GitHub)" <gi...@apache.org>.
manisin commented on PR #7004:
URL: https://github.com/apache/iceberg/pull/7004#issuecomment-1467126295
> overall this LGTM now. Would be good to setup the yml for running the integration test and the secrets in GH so that we could actually see whether the tests pass
Thanks for the review and suggestions @nastra! It would take some wall clock time to get through internal review process for sharing a deployment and onboarding the secrets to github. So, the creation of a new workflow via yml would be taken up in a separate PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org