AWS: Fix GlueCatalog: Add suppressed exception when rollback fails. Throw iceberg AlreadyExistsException rather than aws version

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());

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;
     }
 

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<String, String> 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())