diff --git a/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java b/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java index cb015b79fb9b..16f772eae19d 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java @@ -344,9 +344,9 @@ public void testRenameTableFailsToCreateNewTable() { glueCatalog.renameTable( TableIdentifier.of(namespace, tableName), TableIdentifier.of(namespace, newTableName))) - .isInstanceOf(software.amazon.awssdk.services.glue.model.AlreadyExistsException.class) + .isInstanceOf(AlreadyExistsException.class) .as("should fail to rename to an existing table") - .hasMessageContaining("Table already exists"); + .hasMessageContaining("table already exists"); // old table can still be read with same metadata Table oldTable = glueCatalog.loadTable(id); assertThat(oldTable.location()).isEqualTo(table.location()); diff --git a/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java b/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java index adbf0a03e6b2..719596644276 100644 --- a/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java +++ b/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java @@ -434,12 +434,17 @@ public void renameTable(TableIdentifier from, TableIdentifier to) { .parameters(fromTable.parameters()) .storageDescriptor(fromTable.storageDescriptor()); - glue.createTable( - CreateTableRequest.builder() - .catalogId(awsProperties.glueCatalogId()) - .databaseName(toTableDbName) - .tableInput(tableInputBuilder.name(toTableName).build()) - .build()); + try { + glue.createTable( + CreateTableRequest.builder() + .catalogId(awsProperties.glueCatalogId()) + .databaseName(toTableDbName) + .tableInput(tableInputBuilder.name(toTableName).build()) + .build()); + } catch (software.amazon.awssdk.services.glue.model.AlreadyExistsException e) { + throw new AlreadyExistsException( + e, "Cannot rename %s to %s because table already exists", from, to); + } LOG.info("created rename destination table {}", to); try { @@ -451,12 +456,16 @@ public void renameTable(TableIdentifier from, TableIdentifier to) { from, to, e); - glue.deleteTable( - DeleteTableRequest.builder() - .catalogId(awsProperties.glueCatalogId()) - .databaseName(toTableDbName) - .name(toTableName) - .build()); + try { + dropTable(to, false); + } catch (Exception rollbackException) { + LOG.error( + "Failed to rollback rename of {} to {}, both tables may exist in Glue", + from, + to, + rollbackException); + e.addSuppressed(rollbackException); + } throw e; } diff --git a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java index 82f7e84d563b..58fa2c3cc510 100644 --- a/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java +++ b/aws/src/test/java/org/apache/iceberg/aws/glue/TestGlueCatalog.java @@ -31,6 +31,7 @@ import org.apache.iceberg.aws.s3.S3FileIOProperties; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.NamespaceNotEmptyException; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -423,6 +424,65 @@ public Object answer(InvocationOnMock invocation) throws Throwable { assertThat(counter.get()).isEqualTo(0); } + private void setupRenameTableMocks() { + Map parameters = Maps.newHashMap(); + parameters.put( + BaseMetastoreTableOperations.TABLE_TYPE_PROP, + BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE); + + Mockito.doReturn( + GetTableResponse.builder() + .table(Table.builder().databaseName("db").name("t").parameters(parameters).build()) + .build()) + .when(glue) + .getTable(Mockito.any(GetTableRequest.class)); + Mockito.doReturn( + GetDatabaseResponse.builder().database(Database.builder().name("db").build()).build()) + .when(glue) + .getDatabase(Mockito.any(GetDatabaseRequest.class)); + } + + @Test + public void testRenameTableDestinationAlreadyExists() { + setupRenameTableMocks(); + Mockito.doThrow( + software.amazon.awssdk.services.glue.model.AlreadyExistsException.builder() + .message("Table already exists") + .build()) + .when(glue) + .createTable(Mockito.any(CreateTableRequest.class)); + + assertThatThrownBy( + () -> + glueCatalog.renameTable( + TableIdentifier.of("db", "t"), TableIdentifier.of("db", "t_renamed"))) + .isInstanceOf(AlreadyExistsException.class) + .hasMessageContaining("table already exists"); + } + + @Test + public void testRenameTableRollbackFailure() { + setupRenameTableMocks(); + Mockito.doReturn(CreateTableResponse.builder().build()) + .when(glue) + .createTable(Mockito.any(CreateTableRequest.class)); + + RuntimeException dropFailure = new RuntimeException("drop failed"); + RuntimeException rollbackFailure = new RuntimeException("rollback failed"); + Mockito.doThrow(dropFailure) + .doThrow(rollbackFailure) + .when(glue) + .deleteTable(Mockito.any(DeleteTableRequest.class)); + + assertThatThrownBy( + () -> + glueCatalog.renameTable( + TableIdentifier.of("db", "t"), TableIdentifier.of("db", "t_renamed"))) + .isSameAs(dropFailure) + .satisfies( + ex -> assertThat(ex.getSuppressed()).hasSize(1).allMatch(s -> s == rollbackFailure)); + } + @Test public void testCreateNamespace() { Mockito.doReturn(CreateDatabaseResponse.builder().build())