Skip to content

Core: Registering tables to nonexistent target namespace leads to metadata deletion in HiveCatalog #13434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hsiang-c wants to merge 8 commits into from
Closed

Core: Registering tables to nonexistent target namespace leads to metadata deletion in HiveCatalog #13434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
90aa75c
Fail table registration when target namespace doesn't exist
hsiang-c Jun 29, 2025
aa1f797
Revert changes to Spark procedure
hsiang-c Jul 2, 2025
25871ba
Check namespace existence before registering tables
hsiang-c Jul 5, 2025
a4ba226
Test catalog implementations not coverted by CatalogTests
hsiang-c Jul 5, 2025
5c1f393
Merge branch 'main' into namespace_check
hsiang-c Jul 5, 2025
6438f16
Align catalog client and server behavior by JDBC strict mode
hsiang-c Jul 6, 2025
8374e71
Make error messages consistent in core module
hsiang-c Jul 6, 2025
58e9711
Turn on strict mode only in the tests of iceberg-open-api module
hsiang-c Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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("-", "");
}
Expand Down
16 changes: 16 additions & 0 deletions aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
}
3 changes: 3 additions & 0 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
15 changes: 15 additions & 0 deletions core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -91,6 +96,16 @@ public Table registerTable(TableIdentifier identifier, String metadataFileLocati
return new BaseTable(ops, fullTableName(name(), identifier), metricsReporter());
}

protected void targetNamespaceExists(TableIdentifier identifier) {
Copy link
Contributor

@nastra nastra Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're adding the namespace check at a central place that now affects all catalogs extending this class. I think the main issue is that not every catalog actually requires an explicit namespace creation before e.g. creating a table inside a namespace or registering a table. That's also why we introduced the requiresNamespaceCreate() flag in the tests.
That being said, I don't think we can actually perform this check here as it should be specific to the respective catalog implementation and depending on whether that catalog implementation actually requires a namespace to be created or not.
For example, the default behavior of the JDBC catalog is to not require a namespace to exist when you create a table. That means registering a table should also not require for that namespace to exist beforehand.
However, if you configure strict-mode, then namespace existence is required, meaning that the targetNamespaceExists check should only be performed when strict-mode is on

Copy link
Contributor Author

@hsiang-c hsiang-c Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nastra Thank you for the feedback and I agree w/ you.

That's also why we introduced the requiresNamespaceCreate() flag in the tests.

I like this idea when I am working on the test.

How about we make the requiresNamespaceCreate() a default method (return false by default) in SupportsNamespaces?

Doing so:

  1. Makes the requirement explicitly per implementation, i.e. promoting this semantics from src/test to src/main.
  2. Skips targetNamespaceExists check if the catalog implementation doesn't require it.

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);
Expand Down
11 changes: 11 additions & 0 deletions core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor Author

@hsiang-c hsiang-c Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the error message b/c CatalogTests.tableCreationWithoutNamespace() expects this format.

    assertThatThrownBy(
            () ->
                catalog().buildTable(TableIdentifier.of("non-existing", "table"), SCHEMA).create())
        .isInstanceOf(NoSuchNamespaceException.class)
        .hasMessageContaining("Namespace does not exist: non-existing");

tableIdentifier, catalogName, namespace);
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/iceberg/jdbc/JdbcViewOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
9 changes: 8 additions & 1 deletion core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -512,7 +519,7 @@ public Table registerTable(
client
.withAuthSession(contextualSession)
.post(
paths.register(ident.namespace()),
paths.register(namespace),
request,
LoadTableResponse.class,
Map.of(),
Expand Down
13 changes: 13 additions & 0 deletions core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 12 additions & 0 deletions core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
38 changes: 37 additions & 1 deletion core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions dell/src/test/java/org/apache/iceberg/dell/ecs/TestEcsCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ public void testRenameTable() {

@Test
public void testRegisterTable() {
ecsCatalog.createNamespace(Namespace.of("a"));
Copy link
Contributor

@nastra nastra Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you elaborate why this change is needed?

Copy link
Contributor Author

@hsiang-c hsiang-c Aug 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is because

  1. EcsCatalog implements SupportsNamespaces and the targetNamespaceExists I introduced in this PR requires an existent namespace.
  2. TestEcsCatalog doesn't implement CatalogTests<EcsCatalog> and it doesn't know about the requiresNamespaceCreate method. Therefore, unlike the testRegisterTable in CatalogTests, this test doesn't create a namespace beforehand so I created it here.
// From CatalogTests
  @Test
  public void testRegisterTable() {
    C catalog = catalog();

    if (requiresNamespaceCreate()) {
      catalog.createNamespace(TABLE.namespace());
    }
    // omitted
}

TableIdentifier identifier = TableIdentifier.of("a", "t1");
ecsCatalog.createTable(identifier, SCHEMA);
Table registeringTable = ecsCatalog.loadTable(identifier);
Expand All @@ -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);
Expand All @@ -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");
}
}
4 changes: 1 addition & 3 deletions open-api/src/test/java/org/apache/iceberg/rest/RESTCompatibilityKitCatalogTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ protected RESTCatalog initCatalog(String catalogName, Map<String, String> additi
@Override
protected boolean requiresNamespaceCreate() {
return PropertyUtil.propertyAsBoolean(
restCatalog.properties(),
RESTCompatibilityKitSuite.RCK_REQUIRES_NAMESPACE_CREATE,
super.requiresNamespaceCreate());
Copy link
Contributor Author

@hsiang-c hsiang-c Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super.requiresNamespaceCreate() is false in parent class.

restCatalog.properties(), RESTCompatibilityKitSuite.RCK_REQUIRES_NAMESPACE_CREATE, true);
}

@Override
Expand Down
14 changes: 14 additions & 0 deletions snowflake/src/test/java/org/apache/iceberg/snowflake/TestSnowflakeCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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");
}
}
31 changes: 31 additions & 0 deletions ...ensions/src/test/java/org/apache/iceberg/spark/extensions/TestRegisterTableProcedure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
}