szehon-ho commented on code in PR #53508:
URL: https://github.com/apache/spark/pull/53508#discussion_r2629151376
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -2183,6 +2183,16 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val VIEW_SCHEMA_EVOLUTION_PRESERVE_USER_COMMENTS =
+ buildConf("spark.sql.view.schemaEvolution.preserveUserComments")
+ .internal()
+ .doc("When enabled, views with SCHEMA EVOLUTION mode will preserve
user-set view comments " +
+ "when the underlying table schema evolves. When disabled, view
comments will be " +
+ "overwritten with table comments on every schema sync.")
+ .version("4.0.0")
Review Comment:
nit: wrong version?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -695,19 +695,36 @@ object ViewSyncSchemaToMetaStore extends (LogicalPlan =>
Unit) {
(field.dataType != planField.dataType ||
field.nullable != planField.nullable ||
(viewSchemaMode == SchemaEvolution && (
- field.getComment() != planField.getComment() ||
- field.name != planField.name)))
+ field.name != planField.name ||
+ // Only trigger redo on comment changes if preserve flag is
disabled.
+
(!session.sessionState.conf.viewSchemaEvolutionPreserveUserComments &&
+ field.getComment() != planField.getComment()))))
}
+ lazy val viewFieldsByName = viewFields.map(f => f.name -> f).toMap
+
if (redo) {
val newSchema = if (viewSchemaMode == SchemaTypeEvolution) {
val newFields = viewQuery.schema.map {
case StructField(name, dataType, nullable, _) =>
StructField(name, dataType, nullable,
- viewFields.find(_.name == name).get.metadata)
+ viewFieldsByName(name).metadata)
+ }
+ StructType(newFields)
+ } else if
(session.sessionState.conf.viewSchemaEvolutionPreserveUserComments) {
+ // Adopt types/nullable/names from query, but preserve view
comments.
+ val newFields = viewQuery.schema.map { planField =>
+ val metadataToUse = viewFieldsByName.get(planField.name) match {
+ case Some(viewField) =>
+ viewField.metadata
+ case None =>
+ planField.metadata
Review Comment:
this should not happen?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala:
##########
@@ -695,19 +695,36 @@ object ViewSyncSchemaToMetaStore extends (LogicalPlan =>
Unit) {
(field.dataType != planField.dataType ||
field.nullable != planField.nullable ||
(viewSchemaMode == SchemaEvolution && (
- field.getComment() != planField.getComment() ||
- field.name != planField.name)))
+ field.name != planField.name ||
+ // Only trigger redo on comment changes if preserve flag is
disabled.
+
(!session.sessionState.conf.viewSchemaEvolutionPreserveUserComments &&
+ field.getComment() != planField.getComment()))))
}
+ lazy val viewFieldsByName = viewFields.map(f => f.name -> f).toMap
+
if (redo) {
val newSchema = if (viewSchemaMode == SchemaTypeEvolution) {
val newFields = viewQuery.schema.map {
case StructField(name, dataType, nullable, _) =>
StructField(name, dataType, nullable,
- viewFields.find(_.name == name).get.metadata)
+ viewFieldsByName(name).metadata)
+ }
Review Comment:
cant we just put if here in this pattern match?
Return StructField with the planField's metadata if the config is enabled?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
