mihailoale-db commented on code in PR #59271:
URL: https://github.com/apache/spark/pull/59271#discussion_r4260515170


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala:
##########
@@ -1759,15 +1759,28 @@ case class Unpivot(
 
   def canBeCoercioned: Boolean = values.exists(_.nonEmpty) &&
     values.exists(_.forall(_.forall(_.resolved))) &&
+    // values are coerced by position, so every value group must have one 
value per value column,
+    // otherwise CheckAnalysis reports UNPIVOT_VALUE_SIZE_MISMATCH
+    values.exists(_.forall(_.length == valueColumnNames.length)) &&
     // when no ids are given, values must be Attributes (column names) to 
allow detecting ids
     // coercion will add aliases, would disallow detecting ids, so defer 
coercion after id detection
     ids.exists(_.forall(_.resolved))
 
-  def valuesTypeCoercioned: Boolean = canBeCoercioned &&
-    // all inner values at position idx must have the same data type
+  def valuesTypeCoercioned: Boolean = canBeCoercioned && {
+    // all inner values at position idx must have the same data type; the 
value column takes the
+    // data type of the first value, so by default the types must be exactly 
equal, including
+    // nested nullability; the legacy mode ignores nested nullability, so the 
value column then
+    // takes the nested nullability of the first value
+    val sameType: (DataType, DataType) => Boolean =
+      if 
(conf.getConf(SQLConf.LEGACY_UNPIVOT_USE_FIRST_VALUE_NESTED_NULLABILITY)) {

Review Comment:
   Done. `UnpivotResolver` now runs `UnpivotTypeCoercion` only when 
`valuesTypeCoercioned` is false, like the fixed-point `UnpivotCoercion` rule. 
It also validates the value sizes before coercing, so a ragged multi-value 
UNPIVOT reports `UNPIVOT_VALUE_SIZE_MISMATCH`
     instead of failing inside coercion. The legacy flag is now applied in 
`UnpivotTransformer`, which both analyzers use. Note that `UnpivotResolver` 
isn't wired into the single-pass `Resolver` in OSS yet, so this path can't be 
tested end to end here.



-- 
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]

Reply via email to