Skip to content
Snippets Groups Projects
Commit f6df609d authored by Daoyuan Wang's avatar Daoyuan Wang Committed by Michael Armbrust
Browse files

[SPARK-4593][SQL] Return null when denominator is 0

SELECT max(1/0) FROM src
would return a very large number, which is obviously not right.
For hive-0.12, hive would return `Infinity` for 1/0, while for hive-0.13.1, it is `NULL` for 1/0.
I think it is better to keep our behavior with newer Hive version.
This PR ensures that when the divider is 0, the result of expression should be NULL, same with hive-0.13.1

Author: Daoyuan Wang <daoyuan.wang@intel.com>

Closes #3443 from adrian-wang/div and squashes the following commits:

2e98677 [Daoyuan Wang] fix code gen for divide 0
85c28ba [Daoyuan Wang] temp
36236a5 [Daoyuan Wang] add test cases
6f5716f [Daoyuan Wang] fix comments
cee92bd [Daoyuan Wang] avoid evaluation 2 times
22ecd9a [Daoyuan Wang] fix style
cf28c58 [Daoyuan Wang] divide fix
2dfe50f [Daoyuan Wang] return null when divider is 0 of Double type
parent 10664276
No related branches found
No related tags found
No related merge requests found
...@@ -153,6 +153,25 @@ abstract class Expression extends TreeNode[Expression] { ...@@ -153,6 +153,25 @@ abstract class Expression extends TreeNode[Expression] {
} }
} }
/**
* Evaluation helper function for 1 Fractional children expression.
* if the expression result is null, the evaluation result should be null.
*/
@inline
protected final def f1(i: Row, e1: Expression, f: ((Fractional[Any], Any) => Any)): Any = {
val evalE1 = e1.eval(i: Row)
if(evalE1 == null) {
null
} else {
e1.dataType match {
case ft: FractionalType =>
f.asInstanceOf[(Fractional[ft.JvmType], ft.JvmType) => ft.JvmType](
ft.fractional, evalE1.asInstanceOf[ft.JvmType])
case other => sys.error(s"Type $other does not support fractional operations")
}
}
}
/** /**
* Evaluation helper function for 2 Integral children expressions. Those expressions are * Evaluation helper function for 2 Integral children expressions. Those expressions are
* supposed to be in the same data type, and also the return type. * supposed to be in the same data type, and also the return type.
...@@ -189,6 +208,28 @@ abstract class Expression extends TreeNode[Expression] { ...@@ -189,6 +208,28 @@ abstract class Expression extends TreeNode[Expression] {
} }
} }
/**
* Evaluation helper function for 1 Integral children expression.
* if the expression result is null, the evaluation result should be null.
*/
@inline
protected final def i1(i: Row, e1: Expression, f: ((Integral[Any], Any) => Any)): Any = {
val evalE1 = e1.eval(i)
if(evalE1 == null) {
null
} else {
e1.dataType match {
case i: IntegralType =>
f.asInstanceOf[(Integral[i.JvmType], i.JvmType) => i.JvmType](
i.integral, evalE1.asInstanceOf[i.JvmType])
case i: FractionalType =>
f.asInstanceOf[(Integral[i.JvmType], i.JvmType) => i.JvmType](
i.asIntegral, evalE1.asInstanceOf[i.JvmType])
case other => sys.error(s"Type $other does not support numeric operations")
}
}
}
/** /**
* Evaluation helper function for 2 Comparable children expressions. Those expressions are * Evaluation helper function for 2 Comparable children expressions. Those expressions are
* supposed to be in the same data type, and the return type should be Integer: * supposed to be in the same data type, and the return type should be Integer:
......
...@@ -105,11 +105,16 @@ case class Multiply(left: Expression, right: Expression) extends BinaryArithmeti ...@@ -105,11 +105,16 @@ case class Multiply(left: Expression, right: Expression) extends BinaryArithmeti
case class Divide(left: Expression, right: Expression) extends BinaryArithmetic { case class Divide(left: Expression, right: Expression) extends BinaryArithmetic {
def symbol = "/" def symbol = "/"
override def nullable = left.nullable || right.nullable || dataType.isInstanceOf[DecimalType] override def nullable = true
override def eval(input: Row): Any = dataType match { override def eval(input: Row): Any = {
case _: FractionalType => f2(input, left, right, _.div(_, _)) val evalE2 = right.eval(input)
case _: IntegralType => i2(input, left , right, _.quot(_, _)) dataType match {
case _ if evalE2 == null => null
case _ if evalE2 == 0 => null
case ft: FractionalType => f1(input, left, _.div(_, evalE2.asInstanceOf[ft.JvmType]))
case it: IntegralType => i1(input, left, _.quot(_, evalE2.asInstanceOf[it.JvmType]))
}
} }
} }
......
...@@ -359,7 +359,24 @@ abstract class CodeGenerator[InType <: AnyRef, OutType <: AnyRef] extends Loggin ...@@ -359,7 +359,24 @@ abstract class CodeGenerator[InType <: AnyRef, OutType <: AnyRef] extends Loggin
case Add(e1, e2) => (e1, e2) evaluate { case (eval1, eval2) => q"$eval1 + $eval2" } case Add(e1, e2) => (e1, e2) evaluate { case (eval1, eval2) => q"$eval1 + $eval2" }
case Subtract(e1, e2) => (e1, e2) evaluate { case (eval1, eval2) => q"$eval1 - $eval2" } case Subtract(e1, e2) => (e1, e2) evaluate { case (eval1, eval2) => q"$eval1 - $eval2" }
case Multiply(e1, e2) => (e1, e2) evaluate { case (eval1, eval2) => q"$eval1 * $eval2" } case Multiply(e1, e2) => (e1, e2) evaluate { case (eval1, eval2) => q"$eval1 * $eval2" }
case Divide(e1, e2) => (e1, e2) evaluate { case (eval1, eval2) => q"$eval1 / $eval2" } case Divide(e1, e2) =>
val eval1 = expressionEvaluator(e1)
val eval2 = expressionEvaluator(e2)
eval1.code ++ eval2.code ++
q"""
var $nullTerm = false
var $primitiveTerm: ${termForType(e1.dataType)} = 0
if (${eval1.nullTerm} || ${eval2.nullTerm} ) {
$nullTerm = true
} else if (${eval2.primitiveTerm} == 0)
$nullTerm = true
else {
$nullTerm = false
$primitiveTerm = ${eval1.primitiveTerm} / ${eval2.primitiveTerm}
}
""".children
case IsNotNull(e) => case IsNotNull(e) =>
val eval = expressionEvaluator(e) val eval = expressionEvaluator(e)
......
...@@ -149,6 +149,21 @@ class ExpressionEvaluationSuite extends FunSuite { ...@@ -149,6 +149,21 @@ class ExpressionEvaluationSuite extends FunSuite {
checkEvaluation(In(Literal(1), Seq(Literal(1), Literal(2))) && In(Literal(2), Seq(Literal(1), Literal(2))), true) checkEvaluation(In(Literal(1), Seq(Literal(1), Literal(2))) && In(Literal(2), Seq(Literal(1), Literal(2))), true)
} }
test("Divide") {
checkEvaluation(Divide(Literal(2), Literal(1)), 2)
checkEvaluation(Divide(Literal(1.0), Literal(2.0)), 0.5)
checkEvaluation(Divide(Literal(1), Literal(2)), 0)
checkEvaluation(Divide(Literal(1), Literal(0)), null)
checkEvaluation(Divide(Literal(1.0), Literal(0.0)), null)
checkEvaluation(Divide(Literal(0.0), Literal(0.0)), null)
checkEvaluation(Divide(Literal(0), Literal(null, IntegerType)), null)
checkEvaluation(Divide(Literal(1), Literal(null, IntegerType)), null)
checkEvaluation(Divide(Literal(null, IntegerType), Literal(0)), null)
checkEvaluation(Divide(Literal(null, DoubleType), Literal(0.0)), null)
checkEvaluation(Divide(Literal(null, IntegerType), Literal(1)), null)
checkEvaluation(Divide(Literal(null, IntegerType), Literal(null, IntegerType)), null)
}
test("INSET") { test("INSET") {
val hS = HashSet[Any]() + 1 + 2 val hS = HashSet[Any]() + 1 + 2
val nS = HashSet[Any]() + 1 + 2 + null val nS = HashSet[Any]() + 1 + 2 + null
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment