[SPARK-8926][SQL] Code review followup.
I merged https://github.com/apache/spark/pull/7303 so it unblocks another PR. This addresses my own code review comment for that PR. Author: Reynold Xin <rxin@databricks.com> Closes #7313 from rxin/adt and squashes the following commits: 7ade82b [Reynold Xin] Fixed unit tests. f8d5533 [Reynold Xin] [SPARK-8926][SQL] Code review followup.
This commit is contained in:
parent
e204d22bb7
commit
a870a82fb6
|
@ -39,8 +39,8 @@ trait ExpectsInputTypes { self: Expression =>
|
||||||
override def checkInputDataTypes(): TypeCheckResult = {
|
override def checkInputDataTypes(): TypeCheckResult = {
|
||||||
val mismatches = children.zip(inputTypes).zipWithIndex.collect {
|
val mismatches = children.zip(inputTypes).zipWithIndex.collect {
|
||||||
case ((child, expected), idx) if !expected.acceptsType(child.dataType) =>
|
case ((child, expected), idx) if !expected.acceptsType(child.dataType) =>
|
||||||
s"Argument ${idx + 1} is expected to be of type ${expected.simpleString}, " +
|
s"argument ${idx + 1} is expected to be of type ${expected.simpleString}, " +
|
||||||
s"however, ${child.prettyString} is of type ${child.dataType.simpleString}."
|
s"however, '${child.prettyString}' is of type ${child.dataType.simpleString}."
|
||||||
}
|
}
|
||||||
|
|
||||||
if (mismatches.isEmpty) {
|
if (mismatches.isEmpty) {
|
||||||
|
|
|
@ -36,12 +36,28 @@ private[sql] abstract class AbstractDataType {
|
||||||
/**
|
/**
|
||||||
* Returns true if this data type is the same type as `other`. This is different that equality
|
* Returns true if this data type is the same type as `other`. This is different that equality
|
||||||
* as equality will also consider data type parametrization, such as decimal precision.
|
* as equality will also consider data type parametrization, such as decimal precision.
|
||||||
|
*
|
||||||
|
* {{{
|
||||||
|
* // this should return true
|
||||||
|
* DecimalType.isSameType(DecimalType(10, 2))
|
||||||
|
*
|
||||||
|
* // this should return false
|
||||||
|
* NumericType.isSameType(DecimalType(10, 2))
|
||||||
|
* }}}
|
||||||
*/
|
*/
|
||||||
private[sql] def isSameType(other: DataType): Boolean
|
private[sql] def isSameType(other: DataType): Boolean
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns true if `other` is an acceptable input type for a function that expectes this,
|
* Returns true if `other` is an acceptable input type for a function that expectes this,
|
||||||
* possibly abstract, DataType.
|
* possibly abstract, DataType.
|
||||||
|
*
|
||||||
|
* {{{
|
||||||
|
* // this should return true
|
||||||
|
* DecimalType.isSameType(DecimalType(10, 2))
|
||||||
|
*
|
||||||
|
* // this should return true as well
|
||||||
|
* NumericType.acceptsType(DecimalType(10, 2))
|
||||||
|
* }}}
|
||||||
*/
|
*/
|
||||||
private[sql] def acceptsType(other: DataType): Boolean = isSameType(other)
|
private[sql] def acceptsType(other: DataType): Boolean = isSameType(other)
|
||||||
|
|
||||||
|
|
|
@ -58,7 +58,7 @@ class AnalysisErrorSuite extends SparkFunSuite with BeforeAndAfter {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
errorMessages.foreach(m => assert(error.getMessage.toLowerCase contains m.toLowerCase))
|
errorMessages.foreach(m => assert(error.getMessage.toLowerCase.contains(m.toLowerCase)))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -68,21 +68,21 @@ class AnalysisErrorSuite extends SparkFunSuite with BeforeAndAfter {
|
||||||
"single invalid type, single arg",
|
"single invalid type, single arg",
|
||||||
testRelation.select(TestFunction(dateLit :: Nil, IntegerType :: Nil).as('a)),
|
testRelation.select(TestFunction(dateLit :: Nil, IntegerType :: Nil).as('a)),
|
||||||
"cannot resolve" :: "testfunction" :: "argument 1" :: "expected to be of type int" ::
|
"cannot resolve" :: "testfunction" :: "argument 1" :: "expected to be of type int" ::
|
||||||
"null is of type date" ::Nil)
|
"'null' is of type date" ::Nil)
|
||||||
|
|
||||||
errorTest(
|
errorTest(
|
||||||
"single invalid type, second arg",
|
"single invalid type, second arg",
|
||||||
testRelation.select(
|
testRelation.select(
|
||||||
TestFunction(dateLit :: dateLit :: Nil, DateType :: IntegerType :: Nil).as('a)),
|
TestFunction(dateLit :: dateLit :: Nil, DateType :: IntegerType :: Nil).as('a)),
|
||||||
"cannot resolve" :: "testfunction" :: "argument 2" :: "expected to be of type int" ::
|
"cannot resolve" :: "testfunction" :: "argument 2" :: "expected to be of type int" ::
|
||||||
"null is of type date" ::Nil)
|
"'null' is of type date" ::Nil)
|
||||||
|
|
||||||
errorTest(
|
errorTest(
|
||||||
"multiple invalid type",
|
"multiple invalid type",
|
||||||
testRelation.select(
|
testRelation.select(
|
||||||
TestFunction(dateLit :: dateLit :: Nil, IntegerType :: IntegerType :: Nil).as('a)),
|
TestFunction(dateLit :: dateLit :: Nil, IntegerType :: IntegerType :: Nil).as('a)),
|
||||||
"cannot resolve" :: "testfunction" :: "argument 1" :: "argument 2" ::
|
"cannot resolve" :: "testfunction" :: "argument 1" :: "argument 2" ::
|
||||||
"expected to be of type int" :: "null is of type date" ::Nil)
|
"expected to be of type int" :: "'null' is of type date" ::Nil)
|
||||||
|
|
||||||
errorTest(
|
errorTest(
|
||||||
"unresolved window function",
|
"unresolved window function",
|
||||||
|
|
|
@ -79,6 +79,7 @@ class HiveTypeCoercionSuite extends PlanTest {
|
||||||
shouldCast(IntegerType, TypeCollection(DecimalType(10, 2), StringType), DecimalType(10, 2))
|
shouldCast(IntegerType, TypeCollection(DecimalType(10, 2), StringType), DecimalType(10, 2))
|
||||||
|
|
||||||
shouldCast(StringType, NumericType, DoubleType)
|
shouldCast(StringType, NumericType, DoubleType)
|
||||||
|
shouldCast(StringType, TypeCollection(NumericType, BinaryType), DoubleType)
|
||||||
|
|
||||||
// NumericType should not be changed when function accepts any of them.
|
// NumericType should not be changed when function accepts any of them.
|
||||||
Seq(ByteType, ShortType, IntegerType, LongType, FloatType, DoubleType,
|
Seq(ByteType, ShortType, IntegerType, LongType, FloatType, DoubleType,
|
||||||
|
|
Loading…
Reference in a new issue