Skip to content

Commit

Permalink
Core: Move table-creation-without-namespace-test to CatalogTests (apa…
Browse files Browse the repository at this point in the history
  • Loading branch information
nastra authored Apr 27, 2023
1 parent 882459d commit 8ca6885
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public boolean dropTable(TableIdentifier tableIdentifier, boolean purge) {
public List<TableIdentifier> listTables(Namespace namespace) {
if (!namespaceExists(namespace) && !namespace.isEmpty()) {
throw new NoSuchNamespaceException(
"Cannot list tables for namespace: Namespace %s does not exist", namespace);
"Cannot list tables for namespace. Namespace does not exist: %s", namespace);
}

return tables.keySet().stream()
Expand All @@ -144,24 +144,24 @@ public void renameTable(TableIdentifier fromTableIdentifier, TableIdentifier toT

if (!namespaceExists(toTableIdentifier.namespace())) {
throw new NoSuchNamespaceException(
"Cannot rename %s to %s: Namespace %s does not exist",
"Cannot rename %s to %s. Namespace does not exist: %s",
fromTableIdentifier, toTableIdentifier, toTableIdentifier.namespace());
}

if (!tables.containsKey(fromTableIdentifier)) {
throw new NoSuchTableException(
"Cannot rename %s to %s: Table does not exist", fromTableIdentifier, toTableIdentifier);
"Cannot rename %s to %s. Table does not exist", fromTableIdentifier, toTableIdentifier);
}

if (tables.containsKey(toTableIdentifier)) {
throw new AlreadyExistsException(
"Cannot rename %s to %s: Table already exists", fromTableIdentifier, toTableIdentifier);
"Cannot rename %s to %s. Table already exists", fromTableIdentifier, toTableIdentifier);
}

String fromLocation = tables.remove(fromTableIdentifier);
Preconditions.checkState(
null != fromLocation,
"Cannot rename from %s to %s: Source table does not exist",
"Cannot rename from %s to %s. Source table does not exist",
fromTableIdentifier,
toTableIdentifier);

Expand All @@ -177,7 +177,7 @@ public void createNamespace(Namespace namespace) {
public void createNamespace(Namespace namespace, Map<String, String> metadata) {
if (namespaceExists(namespace)) {
throw new AlreadyExistsException(
"Cannot create namespace %s: Namespace already exists", namespace);
"Cannot create namespace %s. Namespace already exists", namespace);
}

namespaces.put(namespace, ImmutableMap.copyOf(metadata));
Expand Down Expand Up @@ -315,7 +315,7 @@ public void doCommit(TableMetadata base, TableMetadata metadata) {

if (null == base && !namespaceExists(tableIdentifier.namespace())) {
throw new NoSuchNamespaceException(
"Cannot create table %s. Namespace %s does not exist",
"Cannot create table %s. Namespace does not exist: %s",
tableIdentifier, tableIdentifier.namespace());
}

Expand Down
12 changes: 12 additions & 0 deletions core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Test;

public abstract class CatalogTests<C extends Catalog & SupportsNamespaces> {
Expand Down Expand Up @@ -2509,6 +2510,17 @@ public void testMetadataFileLocationsRemovalAfterCommit() {
}
}

@Test
public void tableCreationWithoutNamespace() {
Assumptions.assumeTrue(requiresNamespaceCreate());

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

private static void assertEmpty(String context, Catalog catalog, Namespace ns) {
try {
Assert.assertEquals(context, 0, catalog.listTables(ns).size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,8 @@
package org.apache.iceberg.inmemory;

import org.apache.iceberg.catalog.CatalogTests;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.exceptions.NoSuchNamespaceException;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public class TestInMemoryCatalog extends CatalogTests<InMemoryCatalog> {
private InMemoryCatalog catalog;
Expand All @@ -45,16 +40,4 @@ protected InMemoryCatalog catalog() {
protected boolean requiresNamespaceCreate() {
return true;
}

@Test
public void tableCreationWithoutNamespace() {
Assumptions.assumeTrue(requiresNamespaceCreate());

// this should be moved to CatalogTests at some point, but TestNessieCatalog currently fails
// with a different exception than we would expect
Assertions.assertThatThrownBy(
() -> catalog().buildTable(TableIdentifier.of("ns", "table"), SCHEMA).create())
.isInstanceOf(NoSuchNamespaceException.class)
.hasMessage("Cannot create table ns.table. Namespace ns does not exist");
}
}

0 comments on commit 8ca6885

Please sign in to comment.