[GitHub] spark pull request #20010: [SPARK-22826][SQL] findWiderTypeForTwo Fails over...

[bdrillard <gi...@git.apache.org>]

Github user bdrillard commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20010#discussion_r159244795
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -148,6 +160,61 @@ object TypeCoercion {
         case (l, r) => None
       }
     
    +  /**
    +   * Case 2 type widening over complex types. `widerTypeFunc` is a function that finds the wider
    +   * type over point types. The `widerTypeFunc` specifies behavior over whether types should be
    +   * promoted to StringType.
    +   */
    +  private def findWiderTypeForTwoComplex(
    +      t1: DataType,
    +      t2: DataType,
    +      widerTypeFunc: (DataType, DataType) => Option[DataType]): Option[DataType] = {
    +    (t1, t2) match {
    +      case (_, _) if t1 == t2 => Some(t1)
    +      case (NullType, _) => Some(t1)
    +      case (_, NullType) => Some(t1)
    +
    +      case (ArrayType(pointType1, nullable1), ArrayType(pointType2, nullable2)) =>
    +        val dataType = widerTypeFunc.apply(pointType1, pointType2)
    +
    +        dataType.map(ArrayType(_, nullable1 || nullable2))
    +
    +      case (MapType(keyType1, valueType1, nullable1), MapType(keyType2, valueType2, nullable2)) =>
    +        val keyType = widerTypeFunc.apply(keyType1, keyType2)
    +        val valueType = widerTypeFunc.apply(valueType1, valueType2)
    +
    +        if (keyType.nonEmpty && valueType.nonEmpty) {
    +          Some(MapType(keyType.get, valueType.get, nullable1 || nullable2))
    +        } else {
    +          None
    +        }
    +
    +      case (StructType(fields1), StructType(fields2)) =>
    +        val fieldTypes = fields1.zip(fields2).map { case (f1, f2) =>
    --- End diff --
    
    @mgaido91 That's seems to be the assumption already made in [`findTightestCommonType`](https://github.com/bdrillard/spark/blob/d42dfa55105c8944b63781fc61b59c98a99338d1/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala#L115). The [`sameType`](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/types/DataType.scala#L83) function on DataType also requires structfields are ordered the same. 
    
    The difference here is that we don't require the structfields strictly have the same type, so we can support widening to LongType, StringType, etc. But we do require the fields 1. have the same order, and 2. have the same name (either with strict case, or ignoring case).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org