From ebafff0b6bc641d6d8cd0437271244d8d11fa2e3 Mon Sep 17 00:00:00 2001
From: Yuming Wang <wgyumg@gmail.com>
Date: Fri, 4 Aug 2017 13:58:03 +0800
Subject: [PATCH 1/2] ACOS(2) and ASIN(2) should be null

---
 .../expressions/mathExpressions.scala         | 35 +++++++++++++++----
 .../expressions/MathExpressionsSuite.scala    |  4 +--
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
index 615256243ae2a..a87a2d963b1d0 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
@@ -102,6 +102,29 @@ abstract class UnaryLogExpression(f: Double => Double, name: String)
   }
 }
 
+abstract class UnaryArcExpression(f: Double => Double, name: String)
+  extends UnaryMathExpression(f, name) {
+
+  override def nullable: Boolean = true
+
+  protected override def nullSafeEval(input: Any): Any = {
+    val d = input.asInstanceOf[Double]
+    if (d < -1 || d > 1) null else f(d)
+  }
+
+  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
+    nullSafeCodeGen(ctx, ev, c =>
+      s"""
+        if ($c < -1 || $c > 1) {
+          ${ev.isNull} = true;
+        } else {
+          ${ev.value} = java.lang.Math.${funcName}($c);
+        }
+      """
+    )
+  }
+}
+
 /**
  * A binary expression specifically for math functions that take two `Double`s as input and returns
  * a `Double`.
@@ -170,29 +193,29 @@ case class Pi() extends LeafMathExpression(math.Pi, "PI")
 
 // scalastyle:off line.size.limit
 @ExpressionDescription(
-  usage = "_FUNC_(expr) - Returns the inverse cosine (a.k.a. arccosine) of `expr` if -1<=`expr`<=1 or NaN otherwise.",
+  usage = "_FUNC_(expr) - Returns the inverse cosine (a.k.a. arccosine) of `expr` if -1<=`expr`<=1 or NULL otherwise.",
   extended = """
     Examples:
       > SELECT _FUNC_(1);
        0.0
       > SELECT _FUNC_(2);
-       NaN
+       NULL
   """)
 // scalastyle:on line.size.limit
-case class Acos(child: Expression) extends UnaryMathExpression(math.acos, "ACOS")
+case class Acos(child: Expression) extends UnaryArcExpression(math.acos, "ACOS")
 
 // scalastyle:off line.size.limit
 @ExpressionDescription(
-  usage = "_FUNC_(expr) - Returns the inverse sine (a.k.a. arcsine) the arc sin of `expr` if -1<=`expr`<=1 or NaN otherwise.",
+  usage = "_FUNC_(expr) - Returns the inverse sine (a.k.a. arcsine) the arc sin of `expr` if -1<=`expr`<=1 or NULL otherwise.",
   extended = """
     Examples:
       > SELECT _FUNC_(0);
        0.0
       > SELECT _FUNC_(2);
-       NaN
+       NULL
   """)
 // scalastyle:on line.size.limit
-case class Asin(child: Expression) extends UnaryMathExpression(math.asin, "ASIN")
+case class Asin(child: Expression) extends UnaryArcExpression(math.asin, "ASIN")
 
 // scalastyle:off line.size.limit
 @ExpressionDescription(
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala
index 9ee777529aeda..0b45c6a346bb0 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala
@@ -190,7 +190,7 @@ class MathExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
 
   test("asin") {
     testUnary(Asin, math.asin, (-10 to 10).map(_ * 0.1))
-    testUnary(Asin, math.asin, (11 to 20).map(_ * 0.1), expectNaN = true)
+    testUnary(Asin, math.asin, (11 to 20).map(_ * 0.1), expectNull = true)
     checkConsistencyBetweenInterpretedAndCodegen(Asin, DoubleType)
   }
 
@@ -206,7 +206,7 @@ class MathExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
 
   test("acos") {
     testUnary(Acos, math.acos, (-10 to 10).map(_ * 0.1))
-    testUnary(Acos, math.acos, (11 to 20).map(_ * 0.1), expectNaN = true)
+    testUnary(Acos, math.acos, (11 to 20).map(_ * 0.1), expectNull = true)
     checkConsistencyBetweenInterpretedAndCodegen(Acos, DoubleType)
   }
 

From 450e0c9840931443f747ef39e77c3f5d49255468 Mon Sep 17 00:00:00 2001
From: Yuming Wang <wgyumg@gmail.com>
Date: Fri, 4 Aug 2017 17:47:53 +0800
Subject: [PATCH 2/2] Fix test error

---
 .../golden/udf_acos-6-4d2bd33cee047e9a8bb740760c7cc3b4          | 2 +-
 .../golden/udf_asin-6-114c8141f1e831c70d70c570f0ae778f          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sql/hive/src/test/resources/golden/udf_acos-6-4d2bd33cee047e9a8bb740760c7cc3b4 b/sql/hive/src/test/resources/golden/udf_acos-6-4d2bd33cee047e9a8bb740760c7cc3b4
index 736991a138745..7951defec192a 100644
--- a/sql/hive/src/test/resources/golden/udf_acos-6-4d2bd33cee047e9a8bb740760c7cc3b4
+++ b/sql/hive/src/test/resources/golden/udf_acos-6-4d2bd33cee047e9a8bb740760c7cc3b4
@@ -1 +1 @@
-NaN
+NULL
diff --git a/sql/hive/src/test/resources/golden/udf_asin-6-114c8141f1e831c70d70c570f0ae778f b/sql/hive/src/test/resources/golden/udf_asin-6-114c8141f1e831c70d70c570f0ae778f
index 736991a138745..7951defec192a 100644
--- a/sql/hive/src/test/resources/golden/udf_asin-6-114c8141f1e831c70d70c570f0ae778f
+++ b/sql/hive/src/test/resources/golden/udf_asin-6-114c8141f1e831c70d70c570f0ae778f
@@ -1 +1 @@
-NaN
+NULL