Skip to content

Core: Remove requiresNamespaceCreate check from TestRESTCatalog #14367

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
wants to merge 1 commit into from
Closed

Core: Remove requiresNamespaceCreate check from TestRESTCatalog #14367

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
80 changes: 20 additions & 60 deletions core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,9 +904,7 @@ public void testTableSnapshotLoading() {
RESTCatalogProperties.SNAPSHOT_LOADING_MODE,
SnapshotMode.REFS.name()));

if (requiresNamespaceCreate()) {
catalog.createNamespace(TABLE.namespace());
}
catalog.createNamespace(TABLE.namespace());
Copy link
Contributor

@nastra nastra Oct 20, 2025

Choose a reason for hiding this comment

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

requiresNamespaceCreate() depends on the backing catalog that is used in testing. I'm ok removing the check but I think one reason that might justify leaving the check in is when there are subclasses of TestRESTCatalog that have requiresNamespaceCreate() return false and allow creating implicit namespaces when the table is first created.
Let's see what others think here

Copy link
Contributor

@huaxingao huaxingao Oct 21, 2025

Choose a reason for hiding this comment

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

The rationale makes sense to me.


// Create a table with multiple snapshots
Table table = catalog.createTable(TABLE, SCHEMA);
Expand Down Expand Up @@ -978,9 +976,7 @@ public void testTableSnapshotLoadingWithDivergedBranches(String formatVersion) {
RESTCatalogProperties.SNAPSHOT_LOADING_MODE,
SnapshotMode.REFS.name()));

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

Table table =
catalog.createTable(
Expand Down Expand Up @@ -1088,9 +1084,7 @@ public void lazySnapshotLoadingWithDivergedHistory() {
RESTCatalogProperties.SNAPSHOT_LOADING_MODE,
SnapshotMode.REFS.name()));

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

Table table =
catalog.createTable(TABLE, SCHEMA, PartitionSpec.unpartitioned(), ImmutableMap.of());
Expand Down Expand Up @@ -1183,9 +1177,7 @@ public void testTableAuth(
required(1, "id", Types.IntegerType.get(), "unique ID"),
required(2, "data", Types.StringType.get()));

if (requiresNamespaceCreate()) {
catalog.createNamespace(TBL.namespace());
}
catalog.createNamespace(TBL.namespace());

Table table = catalog.createTable(TBL, expectedSchema);
assertThat(table.schema().asStruct())
Expand Down Expand Up @@ -2095,9 +2087,7 @@ public void diffAgainstSingleTable() {
Namespace namespace = Namespace.of("namespace");
TableIdentifier identifier = TableIdentifier.of(namespace, "multipleDiffsAgainstSingleTable");

if (requiresNamespaceCreate()) {
catalog().createNamespace(namespace);
}
catalog().createNamespace(namespace);

Table table = catalog().buildTable(identifier, SCHEMA).create();
Transaction transaction = table.newTransaction();
Expand Down Expand Up @@ -2131,9 +2121,7 @@ public void multipleDiffsAgainstMultipleTables() {
TableIdentifier identifier1 = TableIdentifier.of(namespace, "multiDiffTable1");
TableIdentifier identifier2 = TableIdentifier.of(namespace, "multiDiffTable2");

if (requiresNamespaceCreate()) {
catalog().createNamespace(namespace);
}
catalog().createNamespace(namespace);

Table table1 = catalog().buildTable(identifier1, SCHEMA).create();
Table table2 = catalog().buildTable(identifier2, SCHEMA).create();
Expand Down Expand Up @@ -2177,9 +2165,7 @@ public void multipleDiffsAgainstMultipleTablesLastFails() {
TableIdentifier identifier1 = TableIdentifier.of(namespace, "multiDiffTable1");
TableIdentifier identifier2 = TableIdentifier.of(namespace, "multiDiffTable2");

if (requiresNamespaceCreate()) {
catalog().createNamespace(namespace);
}
catalog().createNamespace(namespace);

catalog().createTable(identifier1, SCHEMA);
catalog().createTable(identifier2, SCHEMA);
Expand Down Expand Up @@ -2365,9 +2351,7 @@ public void testCleanupUncommitedFilesForCleanableFailures() {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog = catalog(adapter);

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

catalog.createTable(TABLE, SCHEMA);
DataFile file =
Expand Down Expand Up @@ -2408,9 +2392,7 @@ public void testNoCleanupForNonCleanableExceptions() {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog = catalog(adapter);

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

catalog.createTable(TABLE, SCHEMA);
Table table = catalog.loadTable(TABLE);
Expand Down Expand Up @@ -2443,9 +2425,7 @@ public void testCleanupCleanableExceptionsCreate() {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog = catalog(adapter);

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

catalog.createTable(TABLE, SCHEMA);
TableIdentifier newTable = TableIdentifier.of(TABLE.namespace(), "some_table");
Expand Down Expand Up @@ -2490,9 +2470,7 @@ public void testNoCleanupForNonCleanableCreateTransaction() {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog = catalog(adapter);

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

catalog.createTable(TABLE, SCHEMA);
TableIdentifier newTable = TableIdentifier.of(TABLE.namespace(), "some_table");
Expand Down Expand Up @@ -2532,9 +2510,7 @@ public void testCleanupCleanableExceptionsReplace() {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog = catalog(adapter);

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

catalog.createTable(TABLE, SCHEMA);
Mockito.doThrow(new NotAuthorizedException("not authorized"))
Expand Down Expand Up @@ -2575,9 +2551,7 @@ public void testNoCleanupForNonCleanableReplaceTransaction() {
RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog));
RESTCatalog catalog = catalog(adapter);

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

catalog.createTable(TABLE, SCHEMA);
Mockito.doThrow(new ServiceFailureException("some service failure"))
Expand Down Expand Up @@ -2778,9 +2752,7 @@ public void testETagWithCreateAndLoadTable() {

RESTCatalog catalog = catalogWithResponseHeaders(respHeaders);

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

catalog.createTable(TABLE, SCHEMA);

Expand All @@ -2799,9 +2771,7 @@ public void testETagWithDifferentTables() {

RESTCatalog catalog = catalogWithResponseHeaders(respHeaders);

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

catalog.createTable(TABLE, SCHEMA);

Expand All @@ -2821,9 +2791,7 @@ public void testETagAfterDataUpdate() {

RESTCatalog catalog = catalogWithResponseHeaders(respHeaders);

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

Table tbl = catalog.createTable(TABLE, SCHEMA);

Expand All @@ -2844,9 +2812,7 @@ public void testETagAfterMetadataOnlyUpdate() {

RESTCatalog catalog = catalogWithResponseHeaders(respHeaders);

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

Table tbl = catalog.createTable(TABLE, SCHEMA);

Expand All @@ -2867,9 +2833,7 @@ public void testETagWithRegisterTable() {

RESTCatalog catalog = catalogWithResponseHeaders(respHeaders);

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

Table tbl = catalog.createTable(TABLE, SCHEMA);

Expand Down Expand Up @@ -2912,9 +2876,7 @@ public void testReconcileOnUnknownSnapshotAddMatchesSnapshotId() {
ImmutableMap.of(
CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO"));

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

catalog.createTable(TABLE, SCHEMA);

Expand Down Expand Up @@ -2972,9 +2934,7 @@ public void testCommitStateUnknownNotReconciled() {
ImmutableMap.of(
CatalogProperties.FILE_IO_IMPL, "org.apache.iceberg.inmemory.InMemoryFileIO"));

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

catalog.createTable(TABLE, SCHEMA);

Expand Down