Moved RemoveNullTypes into ImplicitTypeCasts.

Author: rxin

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercion.scala

@@ -36,7 +36,6 @@ object HiveTypeCoercion {
   val typeCoercionRules =
     PropagateTypes ::
       InConversion ::
-      RemoveNullTypes ::
       WidenTypes ::
       PromoteStrings ::
       DecimalPrecision ::
@@ -148,47 +147,6 @@ object HiveTypeCoercion {
     }
   }
 
-  /**
-   * Removes [[NullType]] (from null literals in SQL) from expressions by adding an explicit cast
-   * into the type the expression supports. This rule is here to avoid handling [[NullType]] in
-   * every expression implementations.
-   *
-   * This works by looking up the expected input types for expressions, and cast [[NullType]]
-   * into some other specific data type that the expression expects. For example, consider [[Add]],
-   * an expression that supports any numeric types.
-   *
-   * When applying this rule on `Add(NullType, NullType)`, the expression will be converted to
-   * `Add(DoubleType, DoubleType)`.
-   */
-  object RemoveNullTypes extends Rule[LogicalPlan] {
-    def apply(plan: LogicalPlan): LogicalPlan = plan transform {
-      case q: LogicalPlan => q transformExpressions {
-        // Skip nodes who's children have not been resolved yet.
-        case e if !e.childrenResolved => e
-
-        // For binary operators whose input types are NullType, cast them to some specific type.
-        case b @ BinaryOperator(left, right)
-          if left.dataType == NullType && right.dataType == NullType &&
-            !b.inputType.acceptsType(NullType) =>
-          // If both inputs are null type (from null literals), cast the null type into some
-          // specific type the expression expects, so expressions don't need to handle NullType
-          val newLeft = Cast(left, b.inputType.defaultConcreteType)
-          val newRight = Cast(right, b.inputType.defaultConcreteType)
-          b.makeCopy(Array(newLeft, newRight))
-
-        case e: ExpectsInputTypes if e.inputTypes.nonEmpty =>
-          val children: Seq[Expression] = e.children.zip(e.inputTypes).map { case (in, expected) =>
-            if (in.dataType == NullType && !expected.acceptsType(NullType)) {
-              Cast(in, expected.defaultConcreteType)
-            } else {
-              in
-            }
-          }
-          e.withNewChildren(children)
-      }
-    }
-  }
-
   /**
    * Widens numeric types and converts strings to numbers when appropriate.
    *
@@ -256,25 +214,6 @@ object HiveTypeCoercion {
           }
 
         Union(newLeft, newRight)
-
-      // Also widen types for BinaryOperator.
-      case q: LogicalPlan => q transformExpressions {
-        // Skip nodes who's children have not been resolved yet.
-        case e if !e.childrenResolved => e
-
-        case b @ BinaryOperator(left, right) if left.dataType != right.dataType =>
-          findTightestCommonTypeOfTwo(left.dataType, right.dataType).map { commonType =>
-            if (b.inputType.acceptsType(commonType)) {
-              // If the expression accepts the tighest common type, cast to that.
-              val newLeft = if (left.dataType == commonType) left else Cast(left, commonType)
-              val newRight = if (right.dataType == commonType) right else Cast(right, commonType)
-              b.makeCopy(Array(newLeft, newRight))
-            } else {
-              // Otherwise, don't do anything with the expression.
-              b
-            }
-          }.getOrElse(b)  // If there is no applicable conversion, leave expression unchanged.
-      }
     }
   }
 
@@ -734,6 +673,40 @@ object HiveTypeCoercion {
           implicitCast(in, expected).getOrElse(in)
         }
         e.withNewChildren(children)
+
+      case e: ExpectsInputTypes if e.inputTypes.nonEmpty =>
+        // Convert NullType into some specific target type for ExpectsInputTypes that don't do
+        // general implicit casting.
+        val children: Seq[Expression] = e.children.zip(e.inputTypes).map { case (in, expected) =>
+          if (in.dataType == NullType && !expected.acceptsType(NullType)) {
+            Cast(in, expected.defaultConcreteType)
+          } else {
+            in
+          }
+        }
+        e.withNewChildren(children)
+
+      case b @ BinaryOperator(left, right)
+        if left.dataType == NullType && right.dataType == NullType &&
+          !b.inputType.acceptsType(NullType) =>
+        // If both inputs are null type (from null literals), cast the null type into some
+        // specific type the expression expects, so expressions don't need to handle NullType
+        val newLeft = Cast(left, b.inputType.defaultConcreteType)
+        val newRight = Cast(right, b.inputType.defaultConcreteType)
+        b.makeCopy(Array(newLeft, newRight))
+
+      case b @ BinaryOperator(left, right) if left.dataType != right.dataType =>
+        findTightestCommonTypeOfTwo(left.dataType, right.dataType).map { commonType =>
+          if (b.inputType.acceptsType(commonType)) {
+            // If the expression accepts the tighest common type, cast to that.
+            val newLeft = if (left.dataType == commonType) left else Cast(left, commonType)
+            val newRight = if (right.dataType == commonType) right else Cast(right, commonType)
+            b.makeCopy(Array(newLeft, newRight))
+          } else {
+            // Otherwise, don't do anything with the expression.
+            b
+          }
+        }.getOrElse(b)  // If there is no applicable conversion, leave expression unchanged.
     }
 
     /**

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercionSuite.scala

@@ -194,26 +194,26 @@ class HiveTypeCoercionSuite extends PlanTest {
       Project(Seq(Alias(transformed, "a")()), testRelation))
   }
 
-  test("RemoveNullTypes for expressions that define ExpectsInputTypes") {
+  test("cast NullType for expresions that implement ExpectsInputTypes") {
     import HiveTypeCoercionSuite._
 
-    ruleTest(HiveTypeCoercion.RemoveNullTypes,
+    ruleTest(HiveTypeCoercion.ImplicitTypeCasts,
       AnyTypeUnaryExpression(Literal.create(null, NullType)),
       AnyTypeUnaryExpression(Literal.create(null, NullType)))
 
-    ruleTest(HiveTypeCoercion.RemoveNullTypes,
+    ruleTest(HiveTypeCoercion.ImplicitTypeCasts,
       NumericTypeUnaryExpression(Literal.create(null, NullType)),
       NumericTypeUnaryExpression(Cast(Literal.create(null, NullType), DoubleType)))
   }
 
-  test("RemoveNullTypes for binary operators") {
+  test("cast NullType for binary operators") {
     import HiveTypeCoercionSuite._
 
-    ruleTest(HiveTypeCoercion.RemoveNullTypes,
+    ruleTest(HiveTypeCoercion.ImplicitTypeCasts,
       AnyTypeBinaryOperator(Literal.create(null, NullType), Literal.create(null, NullType)),
       AnyTypeBinaryOperator(Literal.create(null, NullType), Literal.create(null, NullType)))
 
-    ruleTest(HiveTypeCoercion.RemoveNullTypes,
+    ruleTest(HiveTypeCoercion.ImplicitTypeCasts,
       NumericTypeBinaryOperator(Literal.create(null, NullType), Literal.create(null, NullType)),
       NumericTypeBinaryOperator(
         Cast(Literal.create(null, NullType), DoubleType),