You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "szehon-ho (via GitHub)" <gi...@apache.org> on 2025/11/21 02:47:59 UTC
[PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source has nested fields [spark]
szehon-ho opened a new pull request, #53149:
URL: https://github.com/apache/spark/pull/53149
### What changes were proposed in this pull request?
Introduce a new flag spark.sql.merge.nested.type.assign.by.field that allows UPDATE SET * action in MERGE INTO to be shorthand to assign every nested struct (ie, UPDATE SET a.b.c = source.a.b.c). This will preserve existing struct field in the target table that has no source equivalent, when the corresponding source struct has less fields than target.
Additional code is added to prevent null expansion in this case (ie, a null source struct expanding to a struct of nulls).
### Why are the changes needed?
Following https://github.com/apache/spark/pull/52866, we now allow MERGE INTO to have a source table struct with less nested fields than target table struct. In this scenario, a user making a UPDATE SET * is the action may want to retain these nested fields in target.
If we interpret UPDATE SET * as shorthand to assign every top-column level field, then the target struct is set to source struct object as is, with missing fields nullified. Instead, they may mean that UPDATE SET * is short-hand to assign every nested struct field (ie, UPDATE SET a.b.c = source.a.b.c), in which case the target struct fields missing in source are retained.
### Does this PR introduce _any_ user-facing change?
No, the support to allow source structs to have less fields than target structs in MERGE INTO is unreleased yet, and in any case there is a flag.
### How was this patch tested?
Unit tests, especially around cases where the source struct is null.
### Was this patch authored or co-authored using generative AI tooling?
No
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2557499167
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6686,15 +6686,27 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
- val MERGE_INTO_SOURCE_NESTED_TYPE_COERCION_ENABLED =
- buildConf("spark.sql.merge.source.nested.type.coercion.enabled")
+ val MERGE_INTO_NESTED_TYPE_COERCION_ENABLED =
+ buildConf("spark.sql.merge.nested.type.coercion.enabled")
.internal()
.doc("If enabled, allow MERGE INTO to coerce source nested types if they have less" +
"nested fields than the target table's nested types.")
.version("4.1.0")
.booleanConf
.createWithDefault(true)
+ val MERGE_INTO_NESTED_TYPE_UPDATE_BY_FIELD =
+ buildConf("spark.sql.merge.nested.type.assign.by.field")
+ .internal()
+ .doc("If enabled and spark.sql.merge.source.nested.type.coercion.enabled is true," +
+ "allow MERGE INTO with UPDATE SET * action to set nested structs field by field. " +
+ "In updated rows, target structs will preserve the original value for fields missing " +
+ "in the the source struct. If disabled, the entire target struct will be replaced, " +
+ "and fields missing in the source struct will be null.")
Review Comment:
that sounds good. yea if Spark usually treats struct field as columns, then makes sense. Yea, MERGE INTO update assigning a struct value smaller than struct key is new feature in 4.1, so let me change it before we release
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2551535766
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6695,6 +6695,18 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val MERGE_INTO_SOURCE_NESTED_TYPE_UPDATE_BY_FIELD =
+ buildConf("spark.sql.merge.nested.type.assign.by.fieldv2")
Review Comment:
ah yes i fixed the config name in latest one, thanks!
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6695,6 +6695,18 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val MERGE_INTO_SOURCE_NESTED_TYPE_UPDATE_BY_FIELD =
+ buildConf("spark.sql.merge.nested.type.assign.by.fieldv2")
Review Comment:
ah yes i fixed the config string in latest one, thanks!
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2557262463
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6686,15 +6686,27 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
- val MERGE_INTO_SOURCE_NESTED_TYPE_COERCION_ENABLED =
- buildConf("spark.sql.merge.source.nested.type.coercion.enabled")
+ val MERGE_INTO_NESTED_TYPE_COERCION_ENABLED =
+ buildConf("spark.sql.merge.nested.type.coercion.enabled")
.internal()
.doc("If enabled, allow MERGE INTO to coerce source nested types if they have less" +
"nested fields than the target table's nested types.")
.version("4.1.0")
.booleanConf
.createWithDefault(true)
+ val MERGE_INTO_NESTED_TYPE_UPDATE_BY_FIELD =
+ buildConf("spark.sql.merge.nested.type.assign.by.field")
+ .internal()
+ .doc("If enabled and spark.sql.merge.source.nested.type.coercion.enabled is true," +
+ "allow MERGE INTO with UPDATE SET * action to set nested structs field by field. " +
+ "In updated rows, target structs will preserve the original value for fields missing " +
+ "in the the source struct. If disabled, the entire target struct will be replaced, " +
+ "and fields missing in the source struct will be null.")
Review Comment:
In Spark, both top-level columns and nested struct fields are first class citizens, I think `UPDATE SET *` should always be recursive and assign values to every leaf fields. Why do we need such a config to switch to an unreasonable behavior that updates the struct column as a whole? MERGE INTO schema evolution is a new feature in 4.1, so this behavior switch is also a new feature and there is no legacy concerns.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #53149:
URL: https://github.com/apache/spark/pull/53149#issuecomment-3565538855
Merged to master/4.1 for Apache Spark 4.1.0.
Happy Thanksgiving, @szehon-ho .
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2549138708
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AssignmentUtils.scala:
##########
@@ -210,13 +241,165 @@ object AssignmentUtils extends SQLConfHelper with CastSupport {
}
}
+ private def applyNestedFieldAssignments(
Review Comment:
Note: this is like applyFieldAssignments below, but recurses
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #53149:
URL: https://github.com/apache/spark/pull/53149#issuecomment-3562199448
@cloud-fan can you help review? Thanks
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2550244249
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -7892,9 +7904,12 @@ class SQLConf extends Serializable with Logging with SqlApiConf {
def legacyXMLParserEnabled: Boolean =
getConf(SQLConf.LEGACY_XML_PARSER_ENABLED)
- def coerceMergeNestedTypes: Boolean =
+ def mergeCoerceNestedTypes: Boolean =
Review Comment:
Oh, you also knew that the naming is weird, don't you, @szehon-ho ? That's the reason you renaming this in this PR.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2549138708
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AssignmentUtils.scala:
##########
@@ -210,13 +241,165 @@ object AssignmentUtils extends SQLConfHelper with CastSupport {
}
}
+ private def applyNestedFieldAssignments(
Review Comment:
Note: this is like applyFieldAssignments above, but recurses into nested structs
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2550218881
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6695,6 +6695,18 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val MERGE_INTO_SOURCE_NESTED_TYPE_UPDATE_BY_FIELD =
+ buildConf("spark.sql.merge.nested.type.assign.by.fieldv2")
Review Comment:
Hi, @szehon-ho . The naming space design looks very weird. Why `nested` is at the different level like the following?
```
spark.sql.merge.nested.type.assign.by.fieldv2
spark.sql.merge.source.nested.type.coercion.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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2550218881
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6695,6 +6695,18 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
+ val MERGE_INTO_SOURCE_NESTED_TYPE_UPDATE_BY_FIELD =
+ buildConf("spark.sql.merge.nested.type.assign.by.fieldv2")
Review Comment:
Hi, @szehon-ho . The naming space design looks weird. Why `nested` is at the different level like the following?
```
spark.sql.merge.nested.type.assign.by.fieldv2
spark.sql.merge.source.nested.type.coercion.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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2557262463
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6686,15 +6686,27 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
- val MERGE_INTO_SOURCE_NESTED_TYPE_COERCION_ENABLED =
- buildConf("spark.sql.merge.source.nested.type.coercion.enabled")
+ val MERGE_INTO_NESTED_TYPE_COERCION_ENABLED =
+ buildConf("spark.sql.merge.nested.type.coercion.enabled")
.internal()
.doc("If enabled, allow MERGE INTO to coerce source nested types if they have less" +
"nested fields than the target table's nested types.")
.version("4.1.0")
.booleanConf
.createWithDefault(true)
+ val MERGE_INTO_NESTED_TYPE_UPDATE_BY_FIELD =
+ buildConf("spark.sql.merge.nested.type.assign.by.field")
+ .internal()
+ .doc("If enabled and spark.sql.merge.source.nested.type.coercion.enabled is true," +
+ "allow MERGE INTO with UPDATE SET * action to set nested structs field by field. " +
+ "In updated rows, target structs will preserve the original value for fields missing " +
+ "in the the source struct. If disabled, the entire target struct will be replaced, " +
+ "and fields missing in the source struct will be null.")
Review Comment:
In Spark, both top-level columns and nested struct fields are first class citizens, I think `UPDATE SET *` should always be recursive and assign values to every fields. Why do we need such a config to switch to an unreasonable behavior that updates the struct column as a whole? MERGE INTO schema evolution is a new feature in 4.1, so this behavior switch is also a new feature and there is no legacy concerns.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2557750691
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -6686,15 +6686,27 @@ object SQLConf {
.booleanConf
.createWithDefault(true)
- val MERGE_INTO_SOURCE_NESTED_TYPE_COERCION_ENABLED =
- buildConf("spark.sql.merge.source.nested.type.coercion.enabled")
+ val MERGE_INTO_NESTED_TYPE_COERCION_ENABLED =
+ buildConf("spark.sql.merge.nested.type.coercion.enabled")
.internal()
.doc("If enabled, allow MERGE INTO to coerce source nested types if they have less" +
"nested fields than the target table's nested types.")
.version("4.1.0")
.booleanConf
.createWithDefault(true)
+ val MERGE_INTO_NESTED_TYPE_UPDATE_BY_FIELD =
+ buildConf("spark.sql.merge.nested.type.assign.by.field")
+ .internal()
+ .doc("If enabled and spark.sql.merge.source.nested.type.coercion.enabled is true," +
+ "allow MERGE INTO with UPDATE SET * action to set nested structs field by field. " +
+ "In updated rows, target structs will preserve the original value for fields missing " +
+ "in the the source struct. If disabled, the entire target struct will be replaced, " +
+ "and fields missing in the source struct will be null.")
Review Comment:
FYI: made a follow up PR: https://github.com/apache/spark/pull/53199
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #53149:
URL: https://github.com/apache/spark/pull/53149#discussion_r2551536621
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -7892,9 +7904,12 @@ class SQLConf extends Serializable with Logging with SqlApiConf {
def legacyXMLParserEnabled: Boolean =
getConf(SQLConf.LEGACY_XML_PARSER_ENABLED)
- def coerceMergeNestedTypes: Boolean =
+ def mergeCoerceNestedTypes: Boolean =
Review Comment:
ah yes, its not strictly related, but realized its better they align.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #53149: [SPARK-54289][SQL] Allow MERGE INTO to preserve existing struct fields for UPDATE SET * when source struct has less nested fields than target struct
URL: https://github.com/apache/spark/pull/53149
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org