[apache#598] improvement(api): introduce UpdateComment in SchemaChang…

…e interface to update comment of schema
SteNicholas · Jan 2, 2024 · 0ae403a · 0ae403a
1 parent cc995db
commit 0ae403a
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 17 deletions.
diff --git a/...talog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java b/...talog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/HiveCatalogOperations.java
@@ -283,13 +283,10 @@ public HiveSchema alterSchema(NameIdentifier ident, SchemaChange... changes)
           ident.name(),
           properties.keySet());
 
-      boolean commentUpdated = false;
       String comment = null;
       for (SchemaChange change : changes) {
         if (change instanceof SchemaChange.UpdateComment) {
-          commentUpdated = true;
           comment = ((SchemaChange.UpdateComment) change).getNewComment();
-          properties.put(COMMENT, comment);
         } else if (change instanceof SchemaChange.SetProperty) {
           properties.put(
               ((SchemaChange.SetProperty) change).getProperty(),
@@ -305,9 +302,7 @@ public HiveSchema alterSchema(NameIdentifier ident, SchemaChange... changes)
       // alter the hive database parameters
       Database alteredDatabase = database.deepCopy();
       alteredDatabase.setParameters(properties);
-      if (commentUpdated) {
-        alteredDatabase.setDescription(comment);
-      }
+      alteredDatabase.setDescription(comment == null ? alteredDatabase.getDescription() : comment);
 
       clientPool.run(
           client -> {

diff --git a/...ain/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/IcebergCatalogOperations.java b/...ain/java/com/datastrato/gravitino/catalog/lakehouse/iceberg/IcebergCatalogOperations.java
@@ -246,13 +246,11 @@ public IcebergSchema alterSchema(NameIdentifier ident, SchemaChange... changes)
       List<String> removals = new ArrayList<>();
       Map<String, String> updates = new HashMap<>();
       Map<String, String> resultProperties = new HashMap<>(metadata);
-      boolean commentUpdated = false;
       String comment = null;
       for (SchemaChange change : changes) {
         if (change instanceof SchemaChange.UpdateComment) {
-          commentUpdated = true;
           comment = ((SchemaChange.UpdateComment) change).getNewComment();
-          updates.put("comment", comment);
+          updates.put(IcebergSchemaPropertiesMetadata.COMMENT, comment);
         } else if (change instanceof SchemaChange.SetProperty) {
           String key = ((SchemaChange.SetProperty) change).getProperty();
           String val = ((SchemaChange.SetProperty) change).getValue();
@@ -271,11 +269,11 @@ public IcebergSchema alterSchema(NameIdentifier ident, SchemaChange... changes)
           new IcebergSchema.Builder()
               .withName(ident.name())
               .withComment(
-                  commentUpdated
-                      ? comment
-                      : Optional.of(response.properties())
+                  comment == null
+                      ? Optional.of(response.properties())
                           .map(map -> map.get(IcebergSchemaPropertiesMetadata.COMMENT))
-                          .orElse(null))
+                          .orElse(null)
+                      : comment)
               .withAuditInfo(AuditInfo.EMPTY)
               .withProperties(resultProperties)
               .build();

diff --git a/docs/manage-metadata-using-gravitino.md b/docs/manage-metadata-using-gravitino.md
@@ -517,6 +517,7 @@ Catalog catalog = gravitinoMetaLake.loadCatalog(NameIdentifier.of("metalake", "h
 SupportsSchemas supportsSchemas = catalog.asSchemas();
 
 Schema schema = supportsSchemas.alterSchema(NameIdentifier.of("metalake", "hive_catalog", "schema"),
+    SchemaChange.updateComment("new comment"),
     SchemaChange.removeProperty("key1"),
     SchemaChange.setProperty("key2", "value2"));
 // ...
@@ -527,10 +528,11 @@ Schema schema = supportsSchemas.alterSchema(NameIdentifier.of("metalake", "hive_
 
 Currently, Gravitino supports the following changes to a schema:
 
-| Supported modification | JSON                                                         | Java                                          |
-|------------------------|--------------------------------------------------------------|-----------------------------------------------|
-| Set a property         | `{"@type":"setProperty","property":"key1","value":"value1"}` | `SchemaChange.setProperty("key1", "value1")`  |
-| Remove a property      | `{"@type":"removeProperty","property":"key1"}`               | `SchemaChange.removeProperty("key1")`         |
+| Supported modification | JSON                                                         | Java                                         |
+|------------------------|--------------------------------------------------------------|----------------------------------------------|
+| Update comment         | `{"@type":"updateComment","newComment":"new_comment"}`       | `SchemaChange.updateComment("new_comment")`  |
+| Set a property         | `{"@type":"setProperty","property":"key1","value":"value1"}` | `SchemaChange.setProperty("key1", "value1")` |
+| Remove a property      | `{"@type":"removeProperty","property":"key1"}`               | `SchemaChange.removeProperty("key1")`        |
 
 ### Drop a schema
 You can remove a schema by sending a `DELETE` request to the `/api/metalakes/{metalake_name}/catalogs/{catalog_name}/schemas/{schema_name}` endpoint or just use the Gravitino Java client. The following is an example of dropping a schema: