[SPARK-17617][SQL] Remainder(%) expression.eval returns incorrect result on double value

clockfly · cloud-fan · commit 8f88412c31dc · 2016-09-21T16:57:44.000+08:00
## What changes were proposed in this pull request? Remainder(%) expression's `eval()` returns incorrect result when the dividend is a big double. The reason is that Remainder converts the double dividend to decimal to do "%", and that lose precision. This bug only affects the `eval()` that is used by constant folding, the codegen path is not impacted. ### Before change ``` scala> -5083676433652386516D % 10 res2: Double = -6.0 scala> spark.sql("select -5083676433652386516D % 10 as a").show +---+ | a| +---+ |0.0| +---+ ``` ### After change ``` scala> spark.sql("select -5083676433652386516D % 10 as a").show +----+ | a| +----+ |-6.0| +----+ ``` ## How was this patch tested? Unit test. Author: Sean Zhong <seanzhong@databricks.com> Closes apache#15171 from clockfly/SPARK-17617. (cherry picked from commit 3977223) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
@@ -273,7 +273,11 @@ case class Remainder(left: Expression, right: Expression) extends BinaryArithmet
       if (input1 == null) {
         null
       } else {
-        integral.rem(input1, input2)
+        input1 match {
+          case d: Double => d % input2.asInstanceOf[java.lang.Double]
+          case f: Float => f % input2.asInstanceOf[java.lang.Float]
+          case _ => integral.rem(input1, input2)
+        }
       }
     }
   }
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
@@ -172,6 +172,17 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper
     // }
   }
 
+  test("SPARK-17617: % (Remainder) double % double on super big double") {
+    val leftDouble = Literal(-5083676433652386516D)
+    val rightDouble = Literal(10D)
+    checkEvaluation(Remainder(leftDouble, rightDouble), -6.0D)
+
+    // Float has smaller precision
+    val leftFloat = Literal(-5083676433652386516F)
+    val rightFloat = Literal(10F)
+    checkEvaluation(Remainder(leftFloat, rightFloat), -2.0F)
+  }
+
   test("Abs") {
     testNumericDataTypes { convert =>
       val input = Literal(convert(1))

Original file line number	Diff line number	Diff line change
`@@ -273,7 +273,11 @@ case class Remainder(left: Expression, right: Expression) extends BinaryArithmet`
`273`	`273`	`if (input1 == null) {`
`274`	`274`	`null`
`275`	`275`	`} else {`
`276`		`- integral.rem(input1, input2)`
	`276`	`+ input1 match {`
	`277`	`+ case d: Double => d % input2.asInstanceOf[java.lang.Double]`
	`278`	`+ case f: Float => f % input2.asInstanceOf[java.lang.Float]`
	`279`	`+ case _ => integral.rem(input1, input2)`
	`280`	`+ }`
`277`	`281`	`}`
`278`	`282`	`}`
`279`	`283`	`}`