From 2058053b0c6e5b1c7e91fa029162f22d109aafb1 Mon Sep 17 00:00:00 2001
From: Akira Ajisaka <akiraaj@amazon.com>
Date: Wed, 15 May 2024 09:08:47 +0900
Subject: [PATCH] AWS: Retain Glue Catalog table description after updating
 Iceberg table (#10199)

---
 .../apache/iceberg/aws/glue/GlueTestBase.java | 29 +++++++++++++++++++
 .../aws/glue/TestGlueCatalogTable.java        | 13 +++++++++
 .../iceberg/aws/glue/GlueTableOperations.java |  3 ++
 3 files changed, 45 insertions(+)

diff --git a/aws/src/integration/java/org/apache/iceberg/aws/glue/GlueTestBase.java b/aws/src/integration/java/org/apache/iceberg/aws/glue/GlueTestBase.java
index 2a810f06508f..ed3a235eb0c8 100644
--- a/aws/src/integration/java/org/apache/iceberg/aws/glue/GlueTestBase.java
+++ b/aws/src/integration/java/org/apache/iceberg/aws/glue/GlueTestBase.java
@@ -39,6 +39,11 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import software.amazon.awssdk.services.glue.GlueClient;
+import software.amazon.awssdk.services.glue.model.GetTableRequest;
+import software.amazon.awssdk.services.glue.model.GetTableResponse;
+import software.amazon.awssdk.services.glue.model.Table;
+import software.amazon.awssdk.services.glue.model.TableInput;
+import software.amazon.awssdk.services.glue.model.UpdateTableRequest;
 import software.amazon.awssdk.services.s3.S3Client;
 
 @SuppressWarnings({"VisibilityModifier", "HideUtilityClassConstructor"})
@@ -129,4 +134,28 @@ public static String createTable(String namespace, String tableName) {
     glueCatalog.createTable(TableIdentifier.of(namespace, tableName), schema, partitionSpec);
     return tableName;
   }
+
+  // Directly call Glue API to update table description
+  public static void updateTableDescription(
+      String namespace, String tableName, String description) {
+    GetTableResponse response =
+        glue.getTable(GetTableRequest.builder().databaseName(namespace).name(tableName).build());
+    Table table = response.table();
+    UpdateTableRequest request =
+        UpdateTableRequest.builder()
+            .catalogId(table.catalogId())
+            .databaseName(table.databaseName())
+            .tableInput(
+                TableInput.builder()
+                    .description(description)
+                    .name(table.name())
+                    .partitionKeys(table.partitionKeys())
+                    .tableType(table.tableType())
+                    .owner(table.owner())
+                    .parameters(table.parameters())
+                    .storageDescriptor(table.storageDescriptor())
+                    .build())
+            .build();
+    glue.updateTable(request);
+  }
 }
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 f6bdd89707dc..6dffdb5b9253 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
@@ -182,6 +182,8 @@ public void testUpdateTable() {
     assertThat(current).isNull();
     // create table, refresh should update
     createTable(namespace, tableName);
+    String description = "test description";
+    updateTableDescription(namespace, tableName, description);
     current = ops.refresh();
     assertThat(current.schema()).asString().isEqualTo(schema.toString());
     assertThat(current.spec()).isEqualTo(partitionSpec);
@@ -206,6 +208,17 @@ public void testUpdateTable() {
         .isEqualTo("EXTERNAL_TABLE");
     assertThat(response.table().storageDescriptor().columns()).hasSameSizeAs(schema.columns());
     assertThat(response.table().partitionKeys()).hasSameSizeAs(partitionSpec.fields());
+    assertThat(response.table().description()).isEqualTo(description);
+
+    String updatedComment = "test updated comment";
+    table
+        .updateProperties()
+        .set(IcebergToGlueConverter.GLUE_DESCRIPTION_KEY, updatedComment)
+        .commit();
+    // check table in Glue
+    response =
+        glue.getTable(GetTableRequest.builder().databaseName(namespace).name(tableName).build());
+    assertThat(response.table().description()).isEqualTo(updatedComment);
   }
 
   @Test
diff --git a/aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java b/aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java
index 6e53e707aa09..aedf78523485 100644
--- a/aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java
+++ b/aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java
@@ -316,6 +316,9 @@ void persistGlueTable(
               .skipArchive(awsProperties.glueCatalogSkipArchive())
               .tableInput(
                   TableInput.builder()
+                      // Call description before applyMutation so that applyMutation overwrites the
+                      // description with the comment specified in the query
+                      .description(glueTable.description())
                       .applyMutation(
                           builder ->
                               IcebergToGlueConverter.setTableInputInformation(builder, metadata))