diff --git a/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java b/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java index 584cdfa4f846..ea5571bb12cc 100644 --- a/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java +++ b/nessie/src/main/java/org/apache/iceberg/nessie/NessieCatalog.java @@ -232,17 +232,32 @@ protected String defaultWarehouseLocation(TableIdentifier table) { @Override public List listTables(Namespace namespace) { - return client.listContents(namespace, Content.Type.ICEBERG_TABLE); + return client.listTables(namespace); } @Override public boolean dropTable(TableIdentifier identifier, boolean purge) { - return dropContent(identifier, Content.Type.ICEBERG_TABLE); + TableReference tableReference = parseTableReference(identifier); + return client + .withReference(tableReference.getReference(), tableReference.getHash()) + .dropTable(identifierWithoutTableReference(identifier, tableReference), false); } @Override public void renameTable(TableIdentifier from, TableIdentifier to) { - renameContent(from, to, Content.Type.ICEBERG_TABLE); + TableReference fromTableReference = parseTableReference(from); + TableReference toTableReference = parseTableReference(to); + + validateReferenceForRename(fromTableReference, toTableReference, Content.Type.ICEBERG_TABLE); + + TableIdentifier fromIdentifier = + NessieUtil.removeCatalogName( + identifierWithoutTableReference(from, fromTableReference), name()); + TableIdentifier toIdentifier = + NessieUtil.removeCatalogName(identifierWithoutTableReference(to, toTableReference), name()); + client + .withReference(fromTableReference.getReference(), fromTableReference.getHash()) + .renameTable(fromIdentifier, toIdentifier); } @Override @@ -337,29 +352,36 @@ protected ViewOperations newViewOps(TableIdentifier identifier) { @Override public List listViews(Namespace namespace) { - return client.listContents(namespace, Content.Type.ICEBERG_VIEW); + return client.listViews(namespace); } @Override public boolean dropView(TableIdentifier identifier) { - return dropContent(identifier, Content.Type.ICEBERG_VIEW); - } - - @Override - public void renameView(TableIdentifier from, TableIdentifier to) { - renameContent(from, to, Content.Type.ICEBERG_VIEW); - } - - private boolean dropContent(TableIdentifier identifier, Content.Type type) { TableReference tableReference = parseTableReference(identifier); return client .withReference(tableReference.getReference(), tableReference.getHash()) - .dropContent(identifierWithoutTableReference(identifier, tableReference), false, type); + .dropView(identifierWithoutTableReference(identifier, tableReference), false); } - private void renameContent(TableIdentifier from, TableIdentifier to, Content.Type type) { + @Override + public void renameView(TableIdentifier from, TableIdentifier to) { TableReference fromTableReference = parseTableReference(from); TableReference toTableReference = parseTableReference(to); + + validateReferenceForRename(fromTableReference, toTableReference, Content.Type.ICEBERG_VIEW); + + TableIdentifier fromIdentifier = + NessieUtil.removeCatalogName( + identifierWithoutTableReference(from, fromTableReference), name()); + TableIdentifier toIdentifier = + NessieUtil.removeCatalogName(identifierWithoutTableReference(to, toTableReference), name()); + client + .withReference(fromTableReference.getReference(), fromTableReference.getHash()) + .renameView(fromIdentifier, toIdentifier); + } + + private void validateReferenceForRename( + TableReference fromTableReference, TableReference toTableReference, Content.Type type) { String fromReference = fromTableReference.hasReference() ? fromTableReference.getReference() @@ -377,15 +399,5 @@ private void renameContent(TableIdentifier from, TableIdentifier to, Content.Typ fromReference, toTableReference.getName(), toReference); - - TableIdentifier fromIdentifier = - NessieUtil.removeCatalogName( - identifierWithoutTableReference(from, fromTableReference), name()); - TableIdentifier toIdentifier = - NessieUtil.removeCatalogName(identifierWithoutTableReference(to, toTableReference), name()); - - client - .withReference(fromTableReference.getReference(), fromTableReference.getHash()) - .renameContent(fromIdentifier, toIdentifier, type); } } diff --git a/nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java b/nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java index fd5adabe36af..8956dcea0c54 100644 --- a/nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java +++ b/nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java @@ -156,7 +156,7 @@ public List listViews(Namespace namespace) { } /** Lists Iceberg table or view from the given namespace */ - protected List listContents(Namespace namespace, Content.Type type) { + private List listContents(Namespace namespace, Content.Type type) { try { return withReference(api.getEntries()).get().getEntries().stream() .filter(namespacePredicate(namespace)) @@ -424,7 +424,7 @@ public void renameView(TableIdentifier from, TableIdentifier to) { renameContent(from, to, Content.Type.ICEBERG_VIEW); } - protected void renameContent(TableIdentifier from, TableIdentifier to, Content.Type type) { + private void renameContent(TableIdentifier from, TableIdentifier to, Content.Type type) { getRef().checkMutable(); IcebergContent existingFromContent = fetchContent(from); @@ -453,16 +453,18 @@ protected void renameContent(TableIdentifier from, TableIdentifier to, Content.T contentType, from, to, getRef().getName()), e); } catch (BaseNessieClientServerException e) { + CommitFailedException commitFailedException = + new CommitFailedException( + e, + "Cannot rename %s '%s' to '%s': the current reference is not up to date.", + contentType, + from, + to); + Optional exception = Optional.empty(); if (e instanceof NessieConflictException) { - NessieUtil.handleExceptionsForCommits(e, getRef().getName(), type); + exception = NessieUtil.handleExceptionsForCommits(e, getRef().getName(), type); } - - throw new CommitFailedException( - e, - "Cannot rename %s '%s' to '%s': the current reference is not up to date.", - contentType, - from, - to); + throw exception.orElse(commitFailedException); } catch (HttpClientException ex) { // Intentionally catch all nessie-client-exceptions here and not just the "timeout" variant // to catch all kinds of network errors (e.g. connection reset). Network code implementation @@ -516,7 +518,7 @@ public boolean dropView(TableIdentifier identifier, boolean purge) { return dropContent(identifier, purge, Content.Type.ICEBERG_VIEW); } - protected boolean dropContent(TableIdentifier identifier, boolean purge, Content.Type type) { + private boolean dropContent(TableIdentifier identifier, boolean purge, Content.Type type) { getRef().checkMutable(); IcebergContent existingContent = fetchContent(identifier); diff --git a/nessie/src/main/java/org/apache/iceberg/nessie/NessieTableOperations.java b/nessie/src/main/java/org/apache/iceberg/nessie/NessieTableOperations.java index ab633471284f..32ec41887939 100644 --- a/nessie/src/main/java/org/apache/iceberg/nessie/NessieTableOperations.java +++ b/nessie/src/main/java/org/apache/iceberg/nessie/NessieTableOperations.java @@ -134,7 +134,13 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) { if (ex instanceof NessieConflictException || ex instanceof NessieNotFoundException) { failure = true; } - NessieUtil.handleExceptionsForCommits(ex, client.refName(), Content.Type.ICEBERG_TABLE); + + NessieUtil.handleExceptionsForCommits(ex, client.refName(), Content.Type.ICEBERG_TABLE) + .ifPresent( + exception -> { + throw exception; + }); + } catch (NessieBadRequestException ex) { failure = true; throw NessieUtil.handleBadRequestForCommit(client, key, Content.Type.ICEBERG_TABLE) diff --git a/nessie/src/main/java/org/apache/iceberg/nessie/NessieUtil.java b/nessie/src/main/java/org/apache/iceberg/nessie/NessieUtil.java index 0c5f5e358edf..4043af7a4f58 100644 --- a/nessie/src/main/java/org/apache/iceberg/nessie/NessieUtil.java +++ b/nessie/src/main/java/org/apache/iceberg/nessie/NessieUtil.java @@ -215,23 +215,26 @@ public static ViewMetadata loadViewMetadata( .build(); } - static void handleExceptionsForCommits(Exception exception, String refName, Content.Type type) { + static Optional handleExceptionsForCommits( + Exception exception, String refName, Content.Type type) { if (exception instanceof NessieConflictException) { if (exception instanceof NessieReferenceConflictException) { // Throws a specialized exception, if possible - NessieUtil.maybeThrowSpecializedException( + return NessieUtil.maybeUseSpecializedException( (NessieReferenceConflictException) exception, type); } - throw new CommitFailedException( - exception, - "Cannot commit: Reference hash is out of date. Update the reference '%s' and try again", - refName); + return Optional.of( + new CommitFailedException( + exception, + "Cannot commit: Reference hash is out of date. Update the reference '%s' and try again", + refName)); } if (exception instanceof NessieNotFoundException) { - throw new RuntimeException( - String.format("Cannot commit: Reference '%s' no longer exists", refName), exception); + return Optional.of( + new RuntimeException( + String.format("Cannot commit: Reference '%s' no longer exists", refName), exception)); } if (exception instanceof HttpClientException) { @@ -239,8 +242,9 @@ static void handleExceptionsForCommits(Exception exception, String refName, Cont // to catch all kinds of network errors (e.g. connection reset). Network code implementation // details and all kinds of network devices can induce unexpected behavior. So better be // safe than sorry. - throw new CommitStateUnknownException(exception); + return Optional.of(new CommitStateUnknownException(exception)); } + return Optional.empty(); } static Optional handleBadRequestForCommit( @@ -269,42 +273,44 @@ static Optional handleBadRequestForCommit( return Optional.empty(); } - private static void maybeThrowSpecializedException( + private static Optional maybeUseSpecializedException( NessieReferenceConflictException ex, Content.Type type) { String contentType = contentTypeString(type); - NessieUtil.extractSingleConflict( + Optional singleConflict = + NessieUtil.extractSingleConflict( ex, EnumSet.of( Conflict.ConflictType.NAMESPACE_ABSENT, Conflict.ConflictType.NAMESPACE_NOT_EMPTY, Conflict.ConflictType.KEY_DOES_NOT_EXIST, - Conflict.ConflictType.KEY_EXISTS)) - .ifPresent( - conflict -> { - switch (conflict.conflictType()) { - case NAMESPACE_ABSENT: - throw new NoSuchNamespaceException( - ex, "Namespace does not exist: %s", conflict.key()); - case NAMESPACE_NOT_EMPTY: - throw new NamespaceNotEmptyException( - ex, "Namespace not empty: %s", conflict.key()); - case KEY_DOES_NOT_EXIST: - if (type == Content.Type.ICEBERG_VIEW) { - throw new NoSuchViewException( - ex, "%s does not exist: %s", contentType, conflict.key()); - } else { - throw new NoSuchTableException( - ex, "%s does not exist: %s", contentType, conflict.key()); - } - case KEY_EXISTS: - throw new AlreadyExistsException( - ex, "%s already exists: %s", contentType, conflict.key()); - default: - // Explicit fall-through - break; - } - }); + Conflict.ConflictType.KEY_EXISTS)); + if (!singleConflict.isPresent()) { + return Optional.empty(); + } + + Conflict conflict = singleConflict.get(); + switch (conflict.conflictType()) { + case NAMESPACE_ABSENT: + return Optional.of( + new NoSuchNamespaceException(ex, "Namespace does not exist: %s", conflict.key())); + case NAMESPACE_NOT_EMPTY: + return Optional.of( + new NamespaceNotEmptyException(ex, "Namespace not empty: %s", conflict.key())); + case KEY_DOES_NOT_EXIST: + if (type == Content.Type.ICEBERG_VIEW) { + return Optional.of( + new NoSuchViewException(ex, "%s does not exist: %s", contentType, conflict.key())); + } else { + return Optional.of( + new NoSuchTableException(ex, "%s does not exist: %s", contentType, conflict.key())); + } + case KEY_EXISTS: + return Optional.of( + new AlreadyExistsException(ex, "%s already exists: %s", contentType, conflict.key())); + default: + return Optional.empty(); + } } static String contentTypeString(Content.Type type) { diff --git a/nessie/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java b/nessie/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java index a348e28fb66f..3e8781280f5b 100644 --- a/nessie/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java +++ b/nessie/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java @@ -113,7 +113,11 @@ public void doCommit(ViewMetadata base, ViewMetadata metadata) { if (ex instanceof NessieConflictException || ex instanceof NessieNotFoundException) { failure = true; } - NessieUtil.handleExceptionsForCommits(ex, client.refName(), Content.Type.ICEBERG_VIEW); + NessieUtil.handleExceptionsForCommits(ex, client.refName(), Content.Type.ICEBERG_VIEW) + .ifPresent( + exception -> { + throw exception; + }); } catch (NessieBadRequestException ex) { failure = true; throw NessieUtil.handleBadRequestForCommit(client, key, Content.Type.ICEBERG_VIEW).orElse(ex); diff --git a/nessie/src/test/java/org/apache/iceberg/nessie/TestNessieView.java b/nessie/src/test/java/org/apache/iceberg/nessie/TestNessieView.java index e134f2d888e6..656363ff072b 100644 --- a/nessie/src/test/java/org/apache/iceberg/nessie/TestNessieView.java +++ b/nessie/src/test/java/org/apache/iceberg/nessie/TestNessieView.java @@ -236,13 +236,9 @@ public void testRenameWithTableReference() throws NessieNotFoundException { TableIdentifier toIdentifier = TableIdentifier.of(VIEW_IDENTIFIER.namespace(), toTableReference.toString()); - View viewBeforeRename = catalog.loadView(fromIdentifier); catalog.renameView(fromIdentifier, toIdentifier); Assertions.assertThat(catalog.viewExists(fromIdentifier)).isFalse(); Assertions.assertThat(catalog.viewExists(toIdentifier)).isTrue(); - View viewAfterRename = catalog.loadView(toIdentifier); - Assertions.assertThat(viewBeforeRename.currentVersion().versionId()) - .isEqualTo(viewAfterRename.currentVersion().versionId()); Assertions.assertThat(catalog.dropView(toIdentifier)).isTrue();