You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/03/17 08:10:42 UTC

[GitHub] [spark] gengliangwang commented on a change in pull request #31859: [SPARK-34769][SQL]AnsiTypeCoercion: return closest convertible type among TypeCollection

gengliangwang commented on a change in pull request #31859:
URL: https://github.com/apache/spark/pull/31859#discussion_r595791842



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AnsiTypeCoercion.scala
##########
@@ -191,9 +194,35 @@ object AnsiTypeCoercion extends TypeCoercionBase {
       case (DateType, TimestampType) => Some(TimestampType)
 
       // When we reach here, input type is not acceptable for any types in this type collection,
-      // try to find the first one we can implicitly cast.
+      // first try to find the all the expected types we can implicitly cast:
+      //   1. if there is no convertible data types, return None;
+      //   2. if there is only one convertible data type, cast input as it;
+      //   3. otherwise if there are multiple convertible data types, find the narrowest common data
+      //      type among them. If there is no such narrowest common data type, return None.
       case (_, TypeCollection(types)) =>
-        types.flatMap(implicitCast(inType, _, isInputFoldable)).headOption
+        // Since Spark contains special objects like `NumericType` and `DecimalType`, which accepts
+        // multiple types and they are `AbstractDataType` instead of `DataType`, here we use the
+        // conversion result their representation.
+        val convertibleTypes = types.flatMap(implicitCast(inType, _, isInputFoldable))
+        if (convertibleTypes.isEmpty) {
+          None
+        } else {
+          // find the narrowest common data type, which can be implicit cast to all other
+          // convertible types.
+          val narrowestCommonType = convertibleTypes.find { dt =>
+            convertibleTypes.forall { target =>
+              implicitCast(dt, target, isInputFoldable = false).isDefined
+            }
+          }

Review comment:
       @maropu I have updated the related context in PR description. This part is quite complicated. 




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

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