Skip to content
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.

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

Core: List namespaces/tables when testing identifier with a dot #11991

Merged
merged 1 commit into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 10 additions & 6 deletions core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ public void testLoadNamespaceMetadata() {

assertThatThrownBy(() -> catalog.loadNamespaceMetadata(NS))
.isInstanceOf(NoSuchNamespaceException.class)
.hasMessageStartingWith("Namespace does not exist: newdb");
.hasMessageStartingWith("Namespace does not exist: %s", NS);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just a separate improvement to make sure that the namespace name isn't hardcoded in the expected error msg when setting NS to something else


catalog.createNamespace(NS);
assertThat(catalog.namespaceExists(NS)).as("Namespace should exist").isTrue();
Expand Down Expand Up @@ -331,7 +331,7 @@ public void testSetNamespacePropertiesNamespaceDoesNotExist() {

assertThatThrownBy(() -> catalog.setProperties(NS, ImmutableMap.of("test", "value")))
.isInstanceOf(NoSuchNamespaceException.class)
.hasMessageStartingWith("Namespace does not exist: newdb");
.hasMessageStartingWith("Namespace does not exist: %s", NS);
}

@Test
Expand Down Expand Up @@ -363,7 +363,7 @@ public void testRemoveNamespacePropertiesNamespaceDoesNotExist() {

assertThatThrownBy(() -> catalog.removeProperties(NS, ImmutableSet.of("a", "b")))
.isInstanceOf(NoSuchNamespaceException.class)
.hasMessageStartingWith("Namespace does not exist: newdb");
.hasMessageStartingWith("Namespace does not exist: %s", NS);
}

@Test
Expand Down Expand Up @@ -499,6 +499,8 @@ public void testNamespaceWithDot() {
catalog.createNamespace(withDot);
assertThat(catalog.namespaceExists(withDot)).as("Namespace should exist").isTrue();

assertThat(catalog.listNamespaces()).contains(withDot);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this reproduces #11990


Map<String, String> properties = catalog.loadNamespaceMetadata(withDot);
assertThat(properties).as("Properties should be accessible").isNotNull();
assertThat(catalog.dropNamespace(withDot)).as("Dropping the namespace should succeed").isTrue();
Expand Down Expand Up @@ -565,15 +567,17 @@ public void testTableNameWithDot() {

C catalog = catalog();

TableIdentifier ident = TableIdentifier.of("ns", "ta.ble");
Namespace namespace = Namespace.of("ns");
TableIdentifier ident = TableIdentifier.of(namespace, "ta.ble");
if (requiresNamespaceCreate()) {
catalog.createNamespace(Namespace.of("ns"));
catalog.createNamespace(namespace);
}

assertThat(catalog.tableExists(ident)).as("Table should not exist").isFalse();

catalog.buildTable(ident, SCHEMA).create();
assertThat(catalog.tableExists(ident)).as("Table should exist").isTrue();
assertThat(catalog.listTables(namespace)).contains(ident);

Table loaded = catalog.loadTable(ident);
assertThat(loaded.schema().asStruct())
Expand Down Expand Up @@ -2280,7 +2284,7 @@ public void testReplaceTransactionRequiresTableExists() {

assertThatThrownBy(() -> catalog.buildTable(TABLE, SCHEMA).replaceTransaction())
.isInstanceOf(NoSuchTableException.class)
.hasMessageStartingWith("Table does not exist: newdb.table");
.hasMessageStartingWith("Table does not exist: %s", TABLE);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ protected boolean supportsEmptyNamespace() {
return true;
}

@Override
protected boolean supportsNamesWithDot() {
// namespaces with a dot are not supported
return false;
}

protected List<String> metadataVersionFiles(String location) {
return Stream.of(new File(location).listFiles())
.filter(file -> !file.isDirectory())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ protected JdbcCatalog initCatalog(String catalogName, Map<String, String> additi
}

@Override
protected boolean supportsNamespaceProperties() {
return true;
protected boolean supportsNamesWithDot() {
// namespaces with a dot are not supported
return false;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,11 @@ protected boolean overridesRequestedLocation() {
RESTCompatibilityKitSuite.RCK_OVERRIDES_REQUESTED_LOCATION,
false);
}

@Override
protected boolean supportsNamesWithDot() {
// underlying JDBC catalog doesn't support namespaces with a dot
return PropertyUtil.propertyAsBoolean(
restCatalog.properties(), RESTCompatibilityKitSuite.RCK_SUPPORTS_NAMES_WITH_DOT, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class RESTCompatibilityKitSuite {
static final String RCK_REQUIRES_NAMESPACE_CREATE = "rck.requires-namespace-create";
static final String RCK_SUPPORTS_SERVERSIDE_RETRY = "rck.supports-serverside-retry";
static final String RCK_OVERRIDES_REQUESTED_LOCATION = "rck.overrides-requested-location";
static final String RCK_SUPPORTS_NAMES_WITH_DOT = "rck.supports-names-with-dot";

protected RESTCompatibilityKitSuite() {}
}