From 3affbd8746aaf3a38aca0a9b65198525b973aa64 Mon Sep 17 00:00:00 2001 From: Wenchen Fan Date: Fri, 29 May 2015 15:31:37 +0800 Subject: [PATCH] more fixes --- .../catalyst/analysis/TypeCheckResult.scala | 4 +- .../sql/catalyst/expressions/arithmetic.scala | 3 +- .../sql/catalyst/expressions/predicates.scala | 5 +- .../ExpressionTypeCheckingSuite.scala | 57 +++++++++++-------- 4 files changed, 41 insertions(+), 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 index 165bbabd06d11..d2474b0f3ad3e 100644 --- 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 index 72dc8cc866797..641baf10ad8f3 100644 --- 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 index 874283c0f5a34..2b03d802c9e51 100644 --- 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/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionTypeCheckingSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionTypeCheckingSuite.scala index c9481a5f96254..fb0fcf2ba2c1b 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionTypeCheckingSuite.scala +++ b/sql/catalyst/src/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