Repository: spark Updated Branches: refs/heads/branch-1.5 58fbdf6f7 -> 6eec04e0a
[SPARK-13806] [SQL] fix rounding mode of negative float/double Round() in database usually round the number up (away from zero), it's different than Math.round() in Java. For example: ``` scala> java.lang.Math.round(-3.5) res3: Long = -3 ``` In Database, we should return -4.0 in this cases. This PR remove the buggy special case for scale=0. Add tests for negative values with tie. Author: Davies Liu <[email protected]> Closes #11894 from davies/fix_round. (cherry picked from commit 4700adb98e4a37c2b0ef7123eca8a9a03bbdbe78) Signed-off-by: Davies Liu <[email protected]> Conflicts: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/6eec04e0 Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/6eec04e0 Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/6eec04e0 Branch: refs/heads/branch-1.5 Commit: 6eec04e0ac5133f3336d5fc67da4a52bb4e80877 Parents: 58fbdf6 Author: Davies Liu <[email protected]> Authored: Tue Mar 22 16:45:20 2016 -0700 Committer: Davies Liu <[email protected]> Committed: Tue Mar 22 16:50:07 2016 -0700 ---------------------------------------------------------------------- .../catalyst/expressions/mathExpressions.scala | 48 ++++++-------------- .../expressions/MathFunctionsSuite.scala | 4 ++ 2 files changed, 19 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/6eec04e0/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---------------------------------------------------------------------- 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 15ceb91..f94ba86 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 @@ -691,7 +691,7 @@ case class Round(child: Expression, scale: Expression) if (f.isNaN || f.isInfinite) { f } else { - BigDecimal(f).setScale(_scale, HALF_UP).toFloat + BigDecimal(f.toDouble).setScale(_scale, HALF_UP).toFloat } case DoubleType => val d = input1.asInstanceOf[Double] @@ -747,39 +747,21 @@ case class Round(child: Expression, scale: Expression) s"${ev.primitive} = ${ce.primitive};" } case FloatType => // if child eval to NaN or Infinity, just return it. - if (_scale == 0) { - s""" - if (Float.isNaN(${ce.primitive}) || Float.isInfinite(${ce.primitive})){ - ${ev.primitive} = ${ce.primitive}; - } else { - ${ev.primitive} = Math.round(${ce.primitive}); - }""" - } else { - s""" - if (Float.isNaN(${ce.primitive}) || Float.isInfinite(${ce.primitive})){ - ${ev.primitive} = ${ce.primitive}; - } else { - ${ev.primitive} = java.math.BigDecimal.valueOf(${ce.primitive}). - setScale(${_scale}, java.math.BigDecimal.ROUND_HALF_UP).floatValue(); - }""" - } + s""" + if (Float.isNaN(${ce.primitive}) || Float.isInfinite(${ce.primitive})){ + ${ev.primitive} = ${ce.primitive}; + } else { + ${ev.primitive} = java.math.BigDecimal.valueOf(${ce.primitive}). + setScale(${_scale}, java.math.BigDecimal.ROUND_HALF_UP).floatValue(); + }""" case DoubleType => // if child eval to NaN or Infinity, just return it. - if (_scale == 0) { - s""" - if (Double.isNaN(${ce.primitive}) || Double.isInfinite(${ce.primitive})){ - ${ev.primitive} = ${ce.primitive}; - } else { - ${ev.primitive} = Math.round(${ce.primitive}); - }""" - } else { - s""" - if (Double.isNaN(${ce.primitive}) || Double.isInfinite(${ce.primitive})){ - ${ev.primitive} = ${ce.primitive}; - } else { - ${ev.primitive} = java.math.BigDecimal.valueOf(${ce.primitive}). - setScale(${_scale}, java.math.BigDecimal.ROUND_HALF_UP).doubleValue(); - }""" - } + s""" + if (Double.isNaN(${ce.primitive}) || Double.isInfinite(${ce.primitive})){ + ${ev.primitive} = ${ce.primitive}; + } else { + ${ev.primitive} = java.math.BigDecimal.valueOf(${ce.primitive}). + setScale(${_scale}, java.math.BigDecimal.ROUND_HALF_UP).doubleValue(); + }""" } if (scaleV == null) { // if scale is null, no need to eval its child at all http://git-wip-us.apache.org/repos/asf/spark/blob/6eec04e0/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala ---------------------------------------------------------------------- diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala index 90c59f2..0031fba 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala @@ -542,5 +542,9 @@ class MathFunctionsSuite extends SparkFunSuite with ExpressionEvalHelper { checkEvaluation(Round(Literal.create(null, dataType), Literal.create(null, IntegerType)), null) } + + checkEvaluation(Round(-3.5, 0), -4.0) + checkEvaluation(Round(-0.35, 1), -0.4) + checkEvaluation(Round(-35, -1), -40) } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
