[SPARK-8280][SPARK-8281][SQL]Handle NaN, null and Infinity in math #6835

yjshen · 2015-06-16T04:37:41Z

Just as their counterparts in Hive:

UDFAsin & UDFAcos would just return NaN if its argument is NaN or its absolute value is greater than 1, therefore I think it's enough to just remove NaN checking before return.

Log* would first check argument <= 0.0 and return null for 0.0 instead of -Infinity

rxin · 2015-06-16T08:12:56Z

Jenkins, ok to test.

SparkQA · 2015-06-16T08:36:33Z

Test build #34973 has finished for PR 6835 at commit 2d1dfc1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class UnaryMathLogExpression(f: Double => Double, name: String)
- case class Log(child: Expression) extends UnaryMathLogExpression(math.log, "LOG")
- case class Log10(child: Expression) extends UnaryMathLogExpression(math.log10, "LOG10")
- case class Log1p(child: Expression) extends UnaryMathLogExpression(math.log1p, "LOG1P")

SparkQA · 2015-06-16T11:42:24Z

Test build #34977 has finished for PR 6835 at commit ef8c28d.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class UnaryMathLogExpression(f: Double => Double, name: String)
- case class Log(child: Expression) extends UnaryMathLogExpression(math.log, "LOG")
- case class Log10(child: Expression) extends UnaryMathLogExpression(math.log10, "LOG10")
- case class Log1p(child: Expression) extends UnaryMathLogExpression(math.log1p, "LOG1P")

rxin · 2015-06-18T06:18:01Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala

+  override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode): String = {
+    val eval = child.gen(ctx)
+    eval.code + s"""
+      boolean ${ev.isNull} = ${eval.isNull} || Double.valueOf(${eval.primitive}) <= 0.0;


isn't this just

${eval.primitive} <= 0.0

instead of calling Double.valueOf?

rxin · 2015-06-18T06:29:17Z

@YijieSHEN it'd make sense to merge this after #6725

It might change your code here.

rxin · 2015-06-18T06:37:53Z

btw I was thinking maybe Log2 and the other log functions can just be child class of the Logarithm class added in #6725?

…null

yjshen · 2015-06-18T15:52:13Z

I've merged #6725 in the latest commit.

Also tried to refactor all the log functions to subclass the Logarithm class added there, but find wried and hard to understand then, so just discard that version.

SparkQA · 2015-06-18T15:56:56Z

Test build #35139 has finished for PR 6835 at commit 4be400a.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class UnaryLogarithmExpression(f: Double => Double, name: String, yAsymptote: Double)
- case class Log(child: Expression) extends UnaryLogarithmExpression(math.log, "LOG", 0.0)
- case class Log10(child: Expression) extends UnaryLogarithmExpression(math.log10, "LOG10", 0.0)
- case class Log1p(child: Expression) extends UnaryLogarithmExpression(math.log1p, "LOG1P", -1.0)

SparkQA · 2015-06-18T16:53:33Z

Test build #35141 has finished for PR 6835 at commit a150de5.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class UnaryLogarithmExpression(f: Double => Double, name: String, yAsymptote: Double)
- case class Log(child: Expression) extends UnaryLogarithmExpression(math.log, "LOG", 0.0)
- case class Log10(child: Expression) extends UnaryLogarithmExpression(math.log10, "LOG10", 0.0)
- case class Log1p(child: Expression) extends UnaryLogarithmExpression(math.log1p, "LOG1P", -1.0)

yjshen · 2015-06-18T18:14:35Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala

+//        math.log(evalE2.asInstanceOf[Double]) / math.log(evalE1.asInstanceOf[Double])
+//      }
+//    }
+//  }


Hive's UDFLog doesn't support base in range (0.0, 1.0], it would just eval to null in this case, just act as the commented out code above.

I'm not sure it we also want to behave like this, any ideas?

we should support these. just remove the commented code, and add to inline comment for log that we support (0.0, 1.0], unlike hive.

SparkQA · 2015-06-18T20:04:37Z

Test build #35150 has finished for PR 6835 at commit f19f651.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class UnaryLogarithmExpression(f: Double => Double, name: String, yAsymptote: Double)
- case class Log(child: Expression) extends UnaryLogarithmExpression(math.log, "LOG", 0.0)
- case class Log10(child: Expression) extends UnaryLogarithmExpression(math.log10, "LOG10", 0.0)
- case class Log1p(child: Expression) extends UnaryLogarithmExpression(math.log1p, "LOG1P", -1.0)

SparkQA · 2015-06-24T05:21:05Z

Test build #35629 has finished for PR 6835 at commit 0c96d86.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class UnaryLogarithmExpression(f: Double => Double, name: String, yAsymptote: Double)
- case class Log(child: Expression) extends UnaryLogarithmExpression(math.log, "LOG", 0.0)
- case class Log10(child: Expression) extends UnaryLogarithmExpression(math.log10, "LOG10", 0.0)
- case class Log1p(child: Expression) extends UnaryLogarithmExpression(math.log1p, "LOG1P", -1.0)

yjshen · 2015-06-24T05:37:08Z

Seems build failure is not related to this PR

yjshen · 2015-06-24T05:38:06Z

retest this please.

JoshRosen · 2015-06-24T05:48:33Z

The MiMa test failure is due to an artifact missing from a Maven staging repository; I have opened #6974 to hotfix.

JoshRosen · 2015-06-24T06:07:48Z

Jenkins, retest this please.

SparkQA · 2015-06-24T08:04:18Z

Test build #35645 has finished for PR 6835 at commit 0c96d86.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2015-07-09T06:22:14Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala

@@ -259,19 +285,14 @@ case class Atan2(left: Expression, right: Expression)
        null
      } else {
        // With codegen, the values returned by -0.0 and 0.0 are different. Handled with +0.0
-        val result = math.atan2(evalE1.asInstanceOf[Double] + 0.0,
+        math.atan2(evalE1.asInstanceOf[Double] + 0.0,


i don't think u need to wrap here.

rxin · 2015-07-16T01:21:08Z

@YijieSHEN can you bring this up to date?

yjshen · 2015-07-16T01:36:47Z

@rxin, do you mind my closing this one and creating a new pr for this? The base code is quite outdated.

rxin · 2015-07-16T01:49:04Z

Not at all. Feel free to close this one and submit a new one.
.

yjshen · 2015-07-16T01:50:21Z

ok, will do soon

yjshen · 2015-07-16T18:59:01Z

close this and continue in #7451

yjshen added 2 commits June 16, 2015 12:21

Enable NaN for acos & asin, handle -Infinity for log

a06c924

Enable udf7, udf_acos, udf_asin

2d1dfc1

log1p(-1) should be null rather than -Infinity

ef8c28d

rxin reviewed Jun 18, 2015
View reviewed changes

yjshen added 2 commits June 18, 2015 15:53

Merge remote-tracking branch 'apache-spark/master' into nan_infinity_…

ec8bee2

…null

Refactor Unary logs as well as fix the binary one

4be400a

remove unnecessary Double.valueOf

307ba7e

Style fix

a150de5

Compare the behavior of binary log with hive

f19f651

yjshen reviewed Jun 18, 2015
View reviewed changes

yjshen mentioned this pull request Jun 23, 2015

[SPARK-8279][SQL]Add math function round #6938

Closed

Merge apache-spark/master

0c96d86

rxin reviewed Jul 9, 2015
View reviewed changes

yjshen closed this Jul 16, 2015

yjshen deleted the nan_infinity_null branch July 20, 2015 02:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-8280][SPARK-8281][SQL]Handle NaN, null and Infinity in math #6835

[SPARK-8280][SPARK-8281][SQL]Handle NaN, null and Infinity in math #6835

yjshen commented Jun 16, 2015

rxin commented Jun 16, 2015

SparkQA commented Jun 16, 2015

SparkQA commented Jun 16, 2015

rxin Jun 18, 2015

rxin commented Jun 18, 2015

rxin commented Jun 18, 2015

yjshen commented Jun 18, 2015

SparkQA commented Jun 18, 2015

SparkQA commented Jun 18, 2015

yjshen Jun 18, 2015

rxin Jul 9, 2015

SparkQA commented Jun 18, 2015

SparkQA commented Jun 24, 2015

yjshen commented Jun 24, 2015

yjshen commented Jun 24, 2015

JoshRosen commented Jun 24, 2015

JoshRosen commented Jun 24, 2015

SparkQA commented Jun 24, 2015

rxin Jul 9, 2015

rxin commented Jul 16, 2015

yjshen commented Jul 16, 2015

rxin commented Jul 16, 2015

yjshen commented Jul 16, 2015

yjshen commented Jul 16, 2015

[SPARK-8280][SPARK-8281][SQL]Handle NaN, null and Infinity in math #6835

[SPARK-8280][SPARK-8281][SQL]Handle NaN, null and Infinity in math #6835

Conversation

yjshen commented Jun 16, 2015

rxin commented Jun 16, 2015

SparkQA commented Jun 16, 2015

SparkQA commented Jun 16, 2015

rxin Jun 18, 2015

Choose a reason for hiding this comment

rxin commented Jun 18, 2015

rxin commented Jun 18, 2015

yjshen commented Jun 18, 2015

SparkQA commented Jun 18, 2015

SparkQA commented Jun 18, 2015

yjshen Jun 18, 2015

Choose a reason for hiding this comment

rxin Jul 9, 2015

Choose a reason for hiding this comment

SparkQA commented Jun 18, 2015

SparkQA commented Jun 24, 2015

yjshen commented Jun 24, 2015

yjshen commented Jun 24, 2015

JoshRosen commented Jun 24, 2015

JoshRosen commented Jun 24, 2015

SparkQA commented Jun 24, 2015

rxin Jul 9, 2015

Choose a reason for hiding this comment

rxin commented Jul 16, 2015

yjshen commented Jul 16, 2015

rxin commented Jul 16, 2015

yjshen commented Jul 16, 2015

yjshen commented Jul 16, 2015