szehon-ho commented on code in PR #50991:
URL: https://github.com/apache/spark/pull/50991#discussion_r2103631127


##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:
##########
@@ -684,11 +684,23 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, 
hadoopConf: Configurat
       } else {
         (oldTableDef.schema, oldTableDef.partitionColumnNames)
       }
+
+      // alter column comments
+      val newCommentMap = tableDefinition.schema.map(f => (f.name, 
f.getComment())).toMap
+      val newSchemaWithComments = newSchema.copy(
+        fields = newSchema.fields.map { f =>
+          newCommentMap.getOrElse(f.name, None) match {
+            case Some(comment) => f.withComment(comment)
+            case _ => f
+          }
+        }
+      )
+
       // // Add old table's owner if we need to restore
       val owner = 
Option(tableDefinition.owner).filter(_.nonEmpty).getOrElse(oldTableDef.owner)
       val newDef = tableDefinition.copy(
         storage = newStorage,
-        schema = newSchema,
+        schema = newSchemaWithComments,

Review Comment:
   But it is true, ideally this method should handle update the schema as well. 
Otherwise the caller needs to make two separate alter calls and its more 
expensive



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to