HyukjinKwon commented on code in PR #59271:
URL: https://github.com/apache/spark/pull/59271#discussion_r4226552659


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala:
##########
@@ -1764,9 +1764,10 @@ case class Unpivot(
     ids.exists(_.forall(_.resolved))
 
   def valuesTypeCoercioned: Boolean = canBeCoercioned &&
-    // all inner values at position idx must have the same data type
+    // all inner values at position idx must have exactly the same data type, 
including nested
+    // nullability, because the value column takes the data type of the first 
value
     values.get.head.zipWithIndex.forall { case (v, idx) =>
-      values.get.tail.forall(vals => 
DataTypeUtils.sameType(vals(idx).dataType, v.dataType))
+      values.get.tail.forall(vals => vals(idx).dataType == v.dataType)

Review Comment:
   **Nit (P3):** With `==` here, `UnpivotCoercion` now also fires for 
multi-value groups whose types differ only in nested nullability. It has no 
group-size check, and `UnpivotTypeCoercion` sizes `valueDataTypes` from the 
head group. So a ragged query such as
   
   ```sql
   SELECT * FROM (SELECT array(1) a, array(cast(null as int)) b, 1 c)
   UNPIVOT ((v1, v2) FOR k IN ((a), (b, c)))
   ```
   
   now throws `IndexOutOfBoundsException` during analysis instead of 
`UNPIVOT_VALUE_SIZE_MISMATCH`, which it raised before because `sameType` 
treated `array<int>` with and without `containsNull` as equal and coercion was 
skipped. Skipping `UnpivotCoercion` unless every value group has 
`valueColumnNames.length` elements would let `CheckAnalysis` report the size 
mismatch again. A regression test for this ragged shape would be useful.
   
   **Verification:**
   - **Regression:** A ragged multi-value UNPIVOT (head group shorter than a 
later group) whose values differ only in nested nullability raises 
UNPIVOT_VALUE_SIZE_MISMATCH with the expected names parameter.
   - **Behavior:** The existing nested-nullability merge tests and the existing 
value-size-mismatch tests still pass unchanged.



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