more fixes

apache · Jun 1, 2015 · 3affbd8 · 3affbd8
1 parent 654d46a
commit 3affbd8
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 28 deletions.
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCheckResult.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCheckResult.scala
@@ -18,7 +18,9 @@
 package org.apache.spark.sql.catalyst.analysis
 
 /**
- * todo
+ * Represents the result of `Expression.checkInputDataTypes`.
+ * We will throw `AnalysisException` in `CheckAnalysis` if error message is not null.
+ *
  */
 class TypeCheckResult(val errorMessage: String) extends AnyVal {
   def hasError: Boolean = errorMessage != null

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
@@ -91,7 +91,8 @@ abstract class BinaryArithmetic extends BinaryExpression {
   override def checkInputDataTypes(): TypeCheckResult = {
     if (left.dataType != right.dataType) {
       TypeCheckResult.fail(
-        s"differing types in ${this.getClass.getSimpleName}, ${left.dataType} != ${right.dataType}")
+        s"differing types in ${this.getClass.getSimpleName} " +
+        s"(${left.dataType} and ${right.dataType}).")
     } else {
       checkTypesInternal(dataType)
     }

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
@@ -175,7 +175,8 @@ abstract class BinaryComparison extends BinaryExpression with Predicate {
   override def checkInputDataTypes(): TypeCheckResult = {
     if (left.dataType != right.dataType) {
       TypeCheckResult.fail(
-        s"differing types in ${this.getClass.getSimpleName}, ${left.dataType} != ${right.dataType}")
+        s"differing types in ${this.getClass.getSimpleName} " +
+        s"(${left.dataType} and ${right.dataType}).")
     } else {
       TypeUtils.checkForOrderingExpr(left.dataType, "operator " + symbol)
     }
@@ -275,7 +276,7 @@ case class If(predicate: Expression, trueValue: Expression, falseValue: Expressi
         s"type of predicate expression in If should be boolean, not ${predicate.dataType}")
     } else if (trueValue.dataType != falseValue.dataType) {
       TypeCheckResult.fail(
-        s"differing types in If, ${trueValue.dataType} != ${falseValue.dataType}")
+        s"differing types in If (${trueValue.dataType} and ${falseValue.dataType}).")
     } else {
       TypeCheckResult.success
     }

diff --git a/...rc/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionTypeCheckingSuite.scala b/...rc/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionTypeCheckingSuite.scala
@@ -48,6 +48,11 @@ class ExpressionTypeCheckingSuite extends FunSuite {
     SimpleAnalyzer.checkAnalysis(analyzed)
   }
 
+  def assertErrorForDifferingTypes(expr: Expression): Unit = {
+    assertError(expr,
+      s"differing types in ${expr.getClass.getSimpleName} (IntegerType and BooleanType).")
+  }
+
   test("check types for unary arithmetic") {
     assertError(UnaryMinus('stringField), "operator - accepts numeric type")
     assertSuccess(Sqrt('stringField)) // We will cast String to Double for sqrt
@@ -65,17 +70,16 @@ class ExpressionTypeCheckingSuite extends FunSuite {
     assertSuccess(Remainder('intField, 'stringField))
     // checkAnalysis(BitwiseAnd('intField, 'stringField))
 
-    def msg(caller: String) = s"differing types in $caller, IntegerType != BooleanType"
-    assertError(Add('intField, 'booleanField), msg("Add"))
-    assertError(Subtract('intField, 'booleanField), msg("Subtract"))
-    assertError(Multiply('intField, 'booleanField), msg("Multiply"))
-    assertError(Divide('intField, 'booleanField), msg("Divide"))
-    assertError(Remainder('intField, 'booleanField), msg("Remainder"))
-    assertError(BitwiseAnd('intField, 'booleanField), msg("BitwiseAnd"))
-    assertError(BitwiseOr('intField, 'booleanField), msg("BitwiseOr"))
-    assertError(BitwiseXor('intField, 'booleanField), msg("BitwiseXor"))
-    assertError(MaxOf('intField, 'booleanField), msg("MaxOf"))
-    assertError(MinOf('intField, 'booleanField), msg("MinOf"))
+    assertErrorForDifferingTypes(Add('intField, 'booleanField))
+    assertErrorForDifferingTypes(Subtract('intField, 'booleanField))
+    assertErrorForDifferingTypes(Multiply('intField, 'booleanField))
+    assertErrorForDifferingTypes(Divide('intField, 'booleanField))
+    assertErrorForDifferingTypes(Remainder('intField, 'booleanField))
+    assertErrorForDifferingTypes(BitwiseAnd('intField, 'booleanField))
+    assertErrorForDifferingTypes(BitwiseOr('intField, 'booleanField))
+    assertErrorForDifferingTypes(BitwiseXor('intField, 'booleanField))
+    assertErrorForDifferingTypes(MaxOf('intField, 'booleanField))
+    assertErrorForDifferingTypes(MinOf('intField, 'booleanField))
 
     assertError(Add('booleanField, 'booleanField), "operator + accepts numeric type")
     assertError(Subtract('booleanField, 'booleanField), "operator - accepts numeric type")
@@ -102,19 +106,24 @@ class ExpressionTypeCheckingSuite extends FunSuite {
     assertSuccess(GreaterThan('intField, 'stringField))
     assertSuccess(GreaterThanOrEqual('intField, 'stringField))
 
-    def msg(caller: String) = s"differing types in $caller, IntegerType != BooleanType"
-    assertError(LessThan('intField, 'booleanField), msg("LessThan"))
-    assertError(LessThanOrEqual('intField, 'booleanField), msg("LessThanOrEqual"))
-    assertError(GreaterThan('intField, 'booleanField), msg("GreaterThan"))
-    assertError(GreaterThanOrEqual('intField, 'booleanField), msg("GreaterThanOrEqual"))
-
-    assertError(LessThan('complexField, 'complexField), "operator < accepts non-complex type")
-    assertError(LessThanOrEqual('complexField, 'complexField), "operator <= accepts non-complex type")
-    assertError(GreaterThan('complexField, 'complexField), "operator > accepts non-complex type")
-    assertError(GreaterThanOrEqual('complexField, 'complexField), "operator >= accepts non-complex type")
-
-    assertError(If('intField, 'stringField, 'stringField), "type of predicate expression in If should be boolean")
-    assertError(If('booleanField, 'intField, 'stringField), "differing types in If, IntegerType != StringType")
+    assertErrorForDifferingTypes(LessThan('intField, 'booleanField))
+    assertErrorForDifferingTypes(LessThanOrEqual('intField, 'booleanField))
+    assertErrorForDifferingTypes(GreaterThan('intField, 'booleanField))
+    assertErrorForDifferingTypes(GreaterThanOrEqual('intField, 'booleanField))
+
+    assertError(
+      LessThan('complexField, 'complexField), "operator < accepts non-complex type")
+    assertError(
+      LessThanOrEqual('complexField, 'complexField), "operator <= accepts non-complex type")
+    assertError(
+      GreaterThan('complexField, 'complexField), "operator > accepts non-complex type")
+    assertError(
+      GreaterThanOrEqual('complexField, 'complexField), "operator >= accepts non-complex type")
+
+    assertError(
+      If('intField, 'stringField, 'stringField),
+      "type of predicate expression in If should be boolean")
+    assertErrorForDifferingTypes(If('booleanField, 'intField, 'booleanField))
 
     // Will write tests for CaseWhen later,
     // as the error reporting of it is not handle by the new interface for now