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

2016-09-21 Thread wenchen
Repository: spark
Updated Branches:
  refs/heads/branch-1.6 8646b84fb -> 8f88412c3


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

## 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 

Closes #15171 from clockfly/SPARK-17617.

(cherry picked from commit 3977223a3268aaf6913a325ee459139a4a302b1c)
Signed-off-by: Wenchen Fan 


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/8f88412c
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/8f88412c
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/8f88412c

Branch: refs/heads/branch-1.6
Commit: 8f88412c31dc840c15df9822638645381c82a2fe
Parents: 8646b84
Author: Sean Zhong 
Authored: Wed Sep 21 16:53:34 2016 +0800
Committer: Wenchen Fan 
Committed: Wed Sep 21 16:57:44 2016 +0800

--
 .../spark/sql/catalyst/expressions/arithmetic.scala  |  6 +-
 .../catalyst/expressions/ArithmeticExpressionSuite.scala | 11 +++
 2 files changed, 16 insertions(+), 1 deletion(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/8f88412c/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
--
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
index cfae285..7905825 100644
--- 
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)
+}
   }
 }
   }

http://git-wip-us.apache.org/repos/asf/spark/blob/8f88412c/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
--
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
index 72285c6..a5930d4 100644
--- 
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))


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org



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

2016-09-21 Thread wenchen
Repository: spark
Updated Branches:
  refs/heads/branch-2.0 726f05716 -> 65295bad9


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

## 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 

Closes #15171 from clockfly/SPARK-17617.

(cherry picked from commit 3977223a3268aaf6913a325ee459139a4a302b1c)
Signed-off-by: Wenchen Fan 


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/65295bad
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/65295bad
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/65295bad

Branch: refs/heads/branch-2.0
Commit: 65295bad9b2a81f2394a52eedeb31da0ed2c4847
Parents: 726f057
Author: Sean Zhong 
Authored: Wed Sep 21 16:53:34 2016 +0800
Committer: Wenchen Fan 
Committed: Wed Sep 21 16:53:55 2016 +0800

--
 .../spark/sql/catalyst/expressions/arithmetic.scala  |  6 +-
 .../catalyst/expressions/ArithmeticExpressionSuite.scala | 11 +++
 2 files changed, 16 insertions(+), 1 deletion(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/65295bad/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
--
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
index fa459aa..01c5d82 100644
--- 
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
@@ -309,7 +309,11 @@ case class Remainder(left: Expression, right: Expression)
   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)
+}
   }
 }
   }

http://git-wip-us.apache.org/repos/asf/spark/blob/65295bad/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
--
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
index 2e37887..069c3b3 100644
--- 
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
@@ -173,6 +173,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))


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org



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

2016-09-21 Thread wenchen
Repository: spark
Updated Branches:
  refs/heads/master 7654385f2 -> 3977223a3


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

## 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 

Closes #15171 from clockfly/SPARK-17617.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/3977223a
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/3977223a
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/3977223a

Branch: refs/heads/master
Commit: 3977223a3268aaf6913a325ee459139a4a302b1c
Parents: 7654385
Author: Sean Zhong 
Authored: Wed Sep 21 16:53:34 2016 +0800
Committer: Wenchen Fan 
Committed: Wed Sep 21 16:53:34 2016 +0800

--
 .../spark/sql/catalyst/expressions/arithmetic.scala  |  6 +-
 .../catalyst/expressions/ArithmeticExpressionSuite.scala | 11 +++
 2 files changed, 16 insertions(+), 1 deletion(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/3977223a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
--
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
index 13e539a..6f3db79 100644
--- 
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
@@ -310,7 +310,11 @@ case class Remainder(left: Expression, right: Expression)
   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)
+}
   }
 }
   }

http://git-wip-us.apache.org/repos/asf/spark/blob/3977223a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
--
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
index 5c98242..0d86efd 100644
--- 
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
@@ -175,6 +175,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))


-
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org