-
Notifications
You must be signed in to change notification settings - Fork 28.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-8280][SPARK-8281][SQL]Handle NaN, null and Infinity in math #7451
Conversation
Log family semantic comparision:
Since we disable udf7 to enable log base between (0.0, 1], we do not have log related comparsion with Hive. So, what about other semantics of |
@@ -387,27 +370,18 @@ case class Atan2(left: Expression, right: Expression) | |||
|
|||
protected override def nullSafeEval(input1: Any, input2: Any): Any = { | |||
// With codegen, the values returned by -0.0 and 0.0 are different. Handled with +0.0 | |||
val result = math.atan2(input1.asInstanceOf[Double] + 0.0, input2.asInstanceOf[Double] + 0.0) | |||
if (result.isNaN) null else result | |||
math.atan2(input1.asInstanceOf[Double] + 0.0, input2.asInstanceOf[Double] + 0.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need + 0.0 here, math.atan2
is calling java.lang.Math.atan2
inside
cc @rxin |
Let's follow Hive for log, and make sure we add a unit test. |
So we behave like Hive except that we do support log base in between (0.0, 1.0], and add a unit test like |
Yup. |
Jenkins, add to whitelist. |
Jenkins, ok to test. |
Jenkins, test this please. |
if ($c1 <= 0.0 || $c2 <= 0.0) { | ||
${ev.isNull} = true; | ||
} else { | ||
${ev.primitive} = java.lang.Math.log($c2) / java.lang.Math.log($c1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missed a space here
Jenkins, retest this please. |
Test build #37596 has finished for PR 7451 at commit
|
Test build #37605 has finished for PR 7451 at commit
|
rebased the code to resolve merge conflicts. |
Jenkins, retest this please. |
Test build #37646 has finished for PR 7451 at commit
|
Oops there is another conflict. I tried resolving this locally but it was a big one. Can you resolve it? |
yes, I've resolved it locally, push here again in a minute. |
Jenkins, retest this please. |
1 similar comment
Jenkins, retest this please. |
Thanks - I've merged this. |
Test build #1097 has finished for PR 7451 at commit
|
Test build #37676 has finished for PR 7451 at commit
|
JIRA:
https://issues.apache.org/jira/browse/SPARK-8280
https://issues.apache.org/jira/browse/SPARK-8281