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