diff --git a/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java b/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java index 2d83582c1337..cc3c8311ee2f 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java @@ -39,6 +39,7 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -404,6 +405,17 @@ public void testRegisterExistingTable() { assertThat(catalog.dropNamespace(namespace)).isTrue(); } + @Test + public void testRegisterTableToNonExistingNamespace() { + TableIdentifier targetIdentifier = TableIdentifier.of("non-existing", "table"); + assertThatThrownBy( + () -> + catalog.registerTable( + targetIdentifier, "table_metadata_loc_from_different_catalogs")) + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageStartingWith("Cannot register table"); + } + private static String genRandomName() { return UUID.randomUUID().toString().replace("-", ""); } diff --git a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java index 2042948eb3c9..7cb687b44be0 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java +++ b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java @@ -32,6 +32,7 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NamespaceNotEmptyException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -679,4 +680,19 @@ public void testTableLevelS3TagProperties() { S3FileIOProperties.S3_TAG_ICEBERG_NAMESPACE), "db"); } + + @Test + public void testRegisterTableToNonExistingNamespace() { + TableIdentifier targetIdentifier = TableIdentifier.of("non_existing", "table"); + Mockito.doThrow(NoSuchNamespaceException.class) + .when(glue) + .getDatabase(Mockito.any(GetDatabaseRequest.class)); + + assertThatThrownBy( + () -> + glueCatalog.registerTable( + targetIdentifier, "table_metadata_loc_from_different_catalogs")) + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageStartingWith("Cannot register table"); + } } diff --git a/build.gradle b/build.gradle index d604298b8de0..45fce39f1368 100644 --- a/build.gradle +++ b/build.gradle @@ -1057,6 +1057,9 @@ project(':iceberg-open-api') { systemProperties System.properties .findAll { k, v -> k.startsWith("rck") } .collectEntries { k, v -> { [(k):v, (k.replaceFirst("rck.", "")):v] }} // strip prefix + + // Turn on strict mode for JdbcCatalog backend in RESTCatalogServer only in :iceberg-open-api module + environment "CATALOG_JDBC_STRICT__MODE", "true" } def restCatalogSpec = "$projectDir/rest-catalog-open-api.yaml" diff --git a/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java b/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java index 29068df380a9..74ad1e433b57 100644 --- a/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java +++ b/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java @@ -22,9 +22,12 @@ import java.io.IOException; import java.util.Map; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.CommitFailedException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.iceberg.io.InputFile; import org.apache.iceberg.metrics.MetricsReporter; @@ -78,6 +81,8 @@ public Table registerTable(TableIdentifier identifier, String metadataFileLocati metadataFileLocation != null && !metadataFileLocation.isEmpty(), "Cannot register an empty metadata file location as a table"); + targetNamespaceExists(identifier); + // Throw an exception if this table already exists in the catalog. if (tableExists(identifier)) { throw new AlreadyExistsException("Table already exists: %s", identifier); @@ -91,6 +96,16 @@ public Table registerTable(TableIdentifier identifier, String metadataFileLocati return new BaseTable(ops, fullTableName(name(), identifier), metricsReporter()); } + protected void targetNamespaceExists(TableIdentifier identifier) { + Namespace namespace = identifier.namespace(); + if (this instanceof SupportsNamespaces + && !(((SupportsNamespaces) this).namespaceExists(namespace))) { + throw new NoSuchNamespaceException( + "Cannot register table %s to catalog %s. Namespace does not exist: %s", + identifier, name(), namespace); + } + } + @Override public TableBuilder buildTable(TableIdentifier identifier, Schema schema) { return new BaseMetastoreCatalogTableBuilder(identifier, schema); diff --git a/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java b/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java index 4d0aa08da2ba..04c1b2d34fa2 100644 --- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java +++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java @@ -389,6 +389,17 @@ public void renameTable(TableIdentifier from, TableIdentifier to) { } } + @Override + public void targetNamespaceExists(TableIdentifier identifier) { + Namespace namespace = identifier.namespace(); + if (PropertyUtil.propertyAsBoolean(catalogProperties, JdbcUtil.STRICT_MODE_PROPERTY, false) + && !JdbcUtil.namespaceExists(catalogName, connections, namespace)) { + throw new NoSuchNamespaceException( + "Cannot register table %s to catalog %s. Namespace does not exist: %s", + identifier, catalogName, namespace); + } + } + @Override public String name() { return catalogName; diff --git a/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java b/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java index 619296ad3336..61adf38e0cdf 100644 --- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java +++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java @@ -173,7 +173,7 @@ private void createTable(String newMetadataLocation) throws SQLException, Interr if (PropertyUtil.propertyAsBoolean(catalogProperties, JdbcUtil.STRICT_MODE_PROPERTY, false) && !JdbcUtil.namespaceExists(catalogName, connections, namespace)) { throw new NoSuchNamespaceException( - "Cannot create table %s in catalog %s. Namespace %s does not exist", + "Cannot create table %s in catalog %s. Namespace does not exist: %s", tableIdentifier, catalogName, namespace); } diff --git a/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java b/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java index 10f46941d694..e02b5dc17d82 100644 --- a/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java +++ b/core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java @@ -180,7 +180,7 @@ private void createView(String newMetadataLocation) throws SQLException, Interru if (PropertyUtil.propertyAsBoolean(catalogProperties, JdbcUtil.STRICT_MODE_PROPERTY, false) && !JdbcUtil.namespaceExists(catalogName, connections, namespace)) { throw new NoSuchNamespaceException( - "Cannot create view %s in catalog %s. Namespace %s does not exist", + "Cannot create view %s in catalog %s. Namespace does not exist: %s", viewIdentifier, catalogName, namespace); } diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java index 4e1c339d1fe9..8c2c0dc75c11 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java @@ -501,6 +501,13 @@ public Table registerTable( "Invalid metadata file location: %s", metadataFileLocation); + Namespace namespace = ident.namespace(); + if (!namespaceExists(context, namespace)) { + throw new NoSuchNamespaceException( + "Cannot register table %s to catalog %s. Namespace does not exist: %s", + ident, name(), namespace); + } + RegisterTableRequest request = ImmutableRegisterTableRequest.builder() .name(ident.name()) @@ -512,7 +519,7 @@ public Table registerTable( client .withAuthSession(contextualSession) .post( - paths.register(ident.namespace()), + paths.register(namespace), request, LoadTableResponse.class, Map.of(), diff --git a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java index c2fd24856fb2..06a5781131ac 100644 --- a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java +++ b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java @@ -3147,6 +3147,19 @@ public void testRegisterTable() { assertThat(catalog.tableExists(TABLE)).isFalse(); } + @Test + public void testRegisterTableToNonExistingNamespace() { + assumeThat(requiresNamespaceCreate()) + .isTrue(); // exclude TestJdbcCatalog and TestJdbcCatalogWithV1Schema + TableIdentifier targetIdentifier = TableIdentifier.of("non_existing", "table"); + assertThatThrownBy( + () -> + catalog() + .registerTable(targetIdentifier, "table_metadata_loc_from_different_catalogs")) + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageStartingWith("Cannot register table"); + } + @Test public void testRegisterExistingTable() { C catalog = catalog(); diff --git a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java index 61f5db9029b4..b2363d0b3846 100644 --- a/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java +++ b/core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java @@ -678,4 +678,16 @@ public void testRegisterExistingTable() throws IOException { .hasMessage("Table already exists: a.t1"); assertThat(catalog.dropTable(identifier)).isTrue(); } + + @Test + public void testRegisterTableToNonexistentDB() throws IOException { + HadoopCatalog catalog = hadoopCatalog(); + TableIdentifier targetIdentifier = TableIdentifier.of("non_existing", "table"); + assertThatThrownBy( + () -> + catalog.registerTable( + targetIdentifier, "table_metadata_loc_from_different_catalogs")) + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageStartingWith("Cannot register table"); + } } diff --git a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java index 0b7315787a26..cb8d42e0e035 100644 --- a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java +++ b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java @@ -953,7 +953,7 @@ public void testCreateTableInNonExistingNamespaceStrictMode() { assertThatThrownBy(() -> jdbcCatalog.createTable(identifier, SCHEMA, PARTITION_SPEC)) .isInstanceOf(NoSuchNamespaceException.class) .hasMessage( - "Cannot create table testDb.ns1.ns2.someTable in catalog strict_jdbc_catalog. Namespace testDb.ns1.ns2 does not exist"); + "Cannot create table testDb.ns1.ns2.someTable in catalog strict_jdbc_catalog. Namespace does not exist: testDb.ns1.ns2"); assertThat(jdbcCatalog.tableExists(identifier)).isFalse(); @@ -1109,6 +1109,42 @@ public void testCommitExceptionWithMessage() { } } + @Test + public void testRegisterTableToNonExistingNamespace() { + try (JdbcCatalog jdbcCatalog = initCatalog("non_strict_jdbc_catalog", ImmutableMap.of())) { + TableIdentifier identifier = TableIdentifier.of("a", "t1"); + jdbcCatalog.createNamespace(identifier.namespace()); + jdbcCatalog.createTable(identifier, SCHEMA); + Table table = jdbcCatalog.loadTable(identifier); + TableOperations ops = ((BaseTable) table).operations(); + String metadataLocation = ops.current().metadataFileLocation(); + + TableIdentifier registeredTableId = TableIdentifier.of("non-existing", "t1"); + Table registeredTable = jdbcCatalog.registerTable(registeredTableId, metadataLocation); + + assertThat(registeredTable).isNotNull(); + assertThat(jdbcCatalog.tableExists(registeredTableId)) + .as("Registered table must exist") + .isTrue(); + } + } + + @Test + public void testRegisterTableToNonExistingNamespaceStrictMode() { + try (JdbcCatalog jdbcCatalog = + initCatalog( + "strict_jdbc_catalog", ImmutableMap.of(JdbcUtil.STRICT_MODE_PROPERTY, "true"))) { + + assertThatThrownBy( + () -> + jdbcCatalog.registerTable( + TableIdentifier.of("non-existing", "t1"), + "table_metadata_from_different_catalogs")) + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageStartingWith("Cannot register table"); + } + } + private String createMetadataLocationViaJdbcCatalog(TableIdentifier identifier) throws SQLException { // temporary connection just to actually create a concrete metadata location diff --git a/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsCatalog.java b/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsCatalog.java index 4714d37d72b9..9191abe13ca9 100644 --- a/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsCatalog.java +++ b/dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsCatalog.java @@ -174,6 +174,7 @@ public void testRenameTable() { @Test public void testRegisterTable() { + ecsCatalog.createNamespace(Namespace.of("a")); TableIdentifier identifier = TableIdentifier.of("a", "t1"); ecsCatalog.createTable(identifier, SCHEMA); Table registeringTable = ecsCatalog.loadTable(identifier); @@ -191,6 +192,7 @@ public void testRegisterTable() { @Test public void testRegisterExistingTable() { + ecsCatalog.createNamespace(Namespace.of("a")); TableIdentifier identifier = TableIdentifier.of("a", "t1"); ecsCatalog.createTable(identifier, SCHEMA); Table registeringTable = ecsCatalog.loadTable(identifier); @@ -201,4 +203,15 @@ public void testRegisterExistingTable() { .hasMessage("Table already exists: a.t1"); assertThat(ecsCatalog.dropTable(identifier, true)).isTrue(); } + + @Test + public void testRegisterTableToNonExistingNamespace() { + TableIdentifier targetIdentifier = TableIdentifier.of("non-existing", "table"); + assertThatThrownBy( + () -> + ecsCatalog.registerTable( + targetIdentifier, "table_metadata_loc_from_different_catalogs")) + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageStartingWith("Cannot register table"); + } } diff --git a/open-api/src/test/java/org/apache/iceberg/rest/RESTCompatibilityKitCatalogTests.java b/open-api/src/test/java/org/apache/iceberg/rest/RESTCompatibilityKitCatalogTests.java index 87ec90663db2..cef22b85153e 100644 --- a/open-api/src/test/java/org/apache/iceberg/rest/RESTCompatibilityKitCatalogTests.java +++ b/open-api/src/test/java/org/apache/iceberg/rest/RESTCompatibilityKitCatalogTests.java @@ -72,9 +72,7 @@ protected RESTCatalog initCatalog(String catalogName, Map additi @Override protected boolean requiresNamespaceCreate() { return PropertyUtil.propertyAsBoolean( - restCatalog.properties(), - RESTCompatibilityKitSuite.RCK_REQUIRES_NAMESPACE_CREATE, - super.requiresNamespaceCreate()); + restCatalog.properties(), RESTCompatibilityKitSuite.RCK_REQUIRES_NAMESPACE_CREATE, true); } @Override diff --git a/snowflake/src/test/java/org/apache/iceberg/snowflake/TestSnowflakeCatalog.java b/snowflake/src/test/java/org/apache/iceberg/snowflake/TestSnowflakeCatalog.java index d16b3e8154a5..6f4254abea8a 100644 --- a/snowflake/src/test/java/org/apache/iceberg/snowflake/TestSnowflakeCatalog.java +++ b/snowflake/src/test/java/org/apache/iceberg/snowflake/TestSnowflakeCatalog.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; import java.util.Map; @@ -30,6 +31,7 @@ import org.apache.iceberg.TableMetadataParser; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.inmemory.InMemoryFileIO; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -295,4 +297,16 @@ public void testSchemaExists() { assertThat(catalog.namespaceExists(Namespace.of("DB_1", "NONEXISTENT_SCHEMA"))).isFalse(); assertThat(catalog.namespaceExists(Namespace.of("NONEXISTENT_DB", "SCHEMA_1"))).isFalse(); } + + @Test + public void testRegisterTableToNonexistentDB() { + String dbName = "NONEXISTENT_DB"; + TableIdentifier targetIdentifier = TableIdentifier.of(dbName, "table"); + assertThatThrownBy( + () -> + catalog.registerTable( + targetIdentifier, "table_metadata_loc_from_different_catalogs")) + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageStartingWith("Cannot register table"); + } } diff --git a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRegisterTableProcedure.java b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRegisterTableProcedure.java index a06a67b7d612..ab930a481e42 100644 --- a/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRegisterTableProcedure.java +++ b/spark/v4.0/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRegisterTableProcedure.java @@ -19,12 +19,14 @@ package org.apache.iceberg.spark.extensions; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.atIndex; import java.util.List; import org.apache.iceberg.ParameterizedTestExtension; import org.apache.iceberg.Table; import org.apache.iceberg.TableUtil; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.spark.Spark3Util; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.catalyst.parser.ParseException; @@ -82,4 +84,33 @@ public void testRegisterTable() throws NoSuchTableException, ParseException { .as("Should have the right datafile count in the procedure result") .contains(originalFileCount, atIndex(2)); } + + @TestTemplate + public void testRegisterTableToNonExistingNamespace() + throws NoSuchTableException, ParseException { + long numRows = 1000; + + sql("CREATE TABLE %s (id int, data string) using ICEBERG", tableName); + spark + .range(0, numRows) + .withColumn("data", functions.col("id").cast(DataTypes.StringType)) + .writeTo(tableName) + .append(); + + Table table = Spark3Util.loadIcebergTable(spark, tableName); + String metadataJson = TableUtil.metadataFileLocation(table); + + String targetNameWithNonexistentNamespace = + (catalogName.equals("spark_catalog") ? "" : catalogName + ".") + + "non_existing_namespace." + + "register_table"; + + assertThatThrownBy( + () -> + sql( + "CALL %s.system.register_table('%s', '%s')", + catalogName, targetNameWithNonexistentNamespace, metadataJson)) + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageContaining("Cannot register table"); + } }