[SPARK-20341][SQL] Support BigInt's value that does not fit in long value range #17684

kiszk · 2017-04-19T12:59:34Z

What changes were proposed in this pull request?

This PR avoids an exception in the case where scala.math.BigInt has a value that does not fit into long value range (e.g. Long.MAX_VALUE+1). When we run the following code by using the current Spark, the following exception is thrown.

This PR keeps the value using BigDecimal if we detect such an overflow case by catching ArithmeticException.

Sample program:

case class BigIntWrapper(value:scala.math.BigInt)```
spark.createDataset(BigIntWrapper(scala.math.BigInt("10000000000000000002"))::Nil).show

Exception:

Error while encoding: java.lang.ArithmeticException: BigInteger out of long range
staticinvoke(class org.apache.spark.sql.types.Decimal$, DecimalType(38,0), apply, assertnotnull(assertnotnull(input[0, org.apache.spark.sql.BigIntWrapper, true])).value, true) AS value#0
java.lang.RuntimeException: Error while encoding: java.lang.ArithmeticException: BigInteger out of long range
staticinvoke(class org.apache.spark.sql.types.Decimal$, DecimalType(38,0), apply, assertnotnull(assertnotnull(input[0, org.apache.spark.sql.BigIntWrapper, true])).value, true) AS value#0
	at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder.toRow(ExpressionEncoder.scala:290)
	at org.apache.spark.sql.SparkSession$$anonfun$2.apply(SparkSession.scala:454)
	at org.apache.spark.sql.SparkSession$$anonfun$2.apply(SparkSession.scala:454)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
	at scala.collection.immutable.List.foreach(List.scala:381)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:234)
	at scala.collection.immutable.List.map(List.scala:285)
	at org.apache.spark.sql.SparkSession.createDataset(SparkSession.scala:454)
	at org.apache.spark.sql.Agg$$anonfun$18.apply$mcV$sp(MySuite.scala:192)
	at org.apache.spark.sql.Agg$$anonfun$18.apply(MySuite.scala:192)
	at org.apache.spark.sql.Agg$$anonfun$18.apply(MySuite.scala:192)
	at org.scalatest.Transformer$$anonfun$apply$1.apply$mcV$sp(Transformer.scala:22)
	at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85)
	at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
	at org.scalatest.Transformer.apply(Transformer.scala:22)
	at org.scalatest.Transformer.apply(Transformer.scala:20)
	at org.scalatest.FunSuiteLike$$anon$1.apply(FunSuiteLike.scala:166)
	at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:68)
	at org.scalatest.FunSuiteLike$class.invokeWithFixture$1(FunSuiteLike.scala:163)
	at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:175)
	at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:175)
	at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
	at org.scalatest.FunSuiteLike$class.runTest(FunSuiteLike.scala:175)
...
Caused by: java.lang.ArithmeticException: BigInteger out of long range
	at java.math.BigInteger.longValueExact(BigInteger.java:4531)
	at org.apache.spark.sql.types.Decimal.set(Decimal.scala:140)
	at org.apache.spark.sql.types.Decimal$.apply(Decimal.scala:434)
	at org.apache.spark.sql.types.Decimal.apply(Decimal.scala)
	at org.apache.spark.sql.catalyst.expressions.GeneratedClass$SpecificUnsafeProjection.apply(Unknown Source)
	at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder.toRow(ExpressionEncoder.scala:287)
	... 59 more

How was this patch tested?

Add new test suite into DecimalSuite

SparkQA · 2017-04-19T15:14:58Z

Test build #75941 has finished for PR 17684 at commit 9a15ba2.

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

gatorsmile

What if we simply use the the set function for BigDecimal?

  def set(bigintval: BigInteger): Decimal = set(BigDecimal(bigintval))

gatorsmile · 2017-04-19T18:40:48Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala

@@ -135,11 +135,17 @@ final class Decimal extends Ordered[Decimal] with Serializable {
   * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.


This line needs an update

gatorsmile · 2017-04-19T18:41:11Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala

+    } catch {
+      case _: ArithmeticException =>
+        set(BigDecimal(bigintval))
+        this


This line is not needed.

kiszk · 2017-04-19T19:30:52Z

To always use BigDecimal can make code simple. However, it always create and keep an additional object (i.e. BigDecimal) even if value can be fit into long value range.
This is a design choice: runtime efficiency with longer code v.s. simpler code with runtime inefficiency.
Is the priority for simple code higher than that for runtime efficiency?

gatorsmile · 2017-04-20T05:39:58Z

Ok, could you write a comment in the code to explain it?

SparkQA · 2017-04-20T12:13:03Z

Test build #75980 has finished for PR 17684 at commit 655e115.

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

kiszk · 2017-04-20T16:15:59Z

Thanks, done.

gatorsmile · 2017-04-21T06:52:23Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala

@@ -132,14 +132,22 @@ final class Decimal extends Ordered[Decimal] with Serializable {
  }

  /**
-   * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.
+   * Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0
+   * if value is fit into long value range. Otherwise, use BigDecimal


How about

If the value is not in the range of long, convert it to BigDecimal and the precision and scale is based on the converted value.

Thanks, updated.

gatorsmile · 2017-04-21T06:52:53Z

LGTM except one comment.

cc @cloud-fan

SparkQA · 2017-04-21T09:22:43Z

Test build #76019 has finished for PR 17684 at commit 6425239.

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

cloud-fan · 2017-04-21T14:25:53Z

LGTM, merging to master/2.2!

…alue range ## What changes were proposed in this pull request? This PR avoids an exception in the case where `scala.math.BigInt` has a value that does not fit into long value range (e.g. `Long.MAX_VALUE+1`). When we run the following code by using the current Spark, the following exception is thrown. This PR keeps the value using `BigDecimal` if we detect such an overflow case by catching `ArithmeticException`. Sample program: ``` case class BigIntWrapper(value:scala.math.BigInt)``` spark.createDataset(BigIntWrapper(scala.math.BigInt("10000000000000000002"))::Nil).show ``` Exception: ``` Error while encoding: java.lang.ArithmeticException: BigInteger out of long range staticinvoke(class org.apache.spark.sql.types.Decimal$, DecimalType(38,0), apply, assertnotnull(assertnotnull(input[0, org.apache.spark.sql.BigIntWrapper, true])).value, true) AS value#0 java.lang.RuntimeException: Error while encoding: java.lang.ArithmeticException: BigInteger out of long range staticinvoke(class org.apache.spark.sql.types.Decimal$, DecimalType(38,0), apply, assertnotnull(assertnotnull(input[0, org.apache.spark.sql.BigIntWrapper, true])).value, true) AS value#0 at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder.toRow(ExpressionEncoder.scala:290) at org.apache.spark.sql.SparkSession$$anonfun$2.apply(SparkSession.scala:454) at org.apache.spark.sql.SparkSession$$anonfun$2.apply(SparkSession.scala:454) at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) at scala.collection.immutable.List.foreach(List.scala:381) at scala.collection.TraversableLike$class.map(TraversableLike.scala:234) at scala.collection.immutable.List.map(List.scala:285) at org.apache.spark.sql.SparkSession.createDataset(SparkSession.scala:454) at org.apache.spark.sql.Agg$$anonfun$18.apply$mcV$sp(MySuite.scala:192) at org.apache.spark.sql.Agg$$anonfun$18.apply(MySuite.scala:192) at org.apache.spark.sql.Agg$$anonfun$18.apply(MySuite.scala:192) at org.scalatest.Transformer$$anonfun$apply$1.apply$mcV$sp(Transformer.scala:22) at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85) at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104) at org.scalatest.Transformer.apply(Transformer.scala:22) at org.scalatest.Transformer.apply(Transformer.scala:20) at org.scalatest.FunSuiteLike$$anon$1.apply(FunSuiteLike.scala:166) at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:68) at org.scalatest.FunSuiteLike$class.invokeWithFixture$1(FunSuiteLike.scala:163) at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:175) at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:175) at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306) at org.scalatest.FunSuiteLike$class.runTest(FunSuiteLike.scala:175) ... Caused by: java.lang.ArithmeticException: BigInteger out of long range at java.math.BigInteger.longValueExact(BigInteger.java:4531) at org.apache.spark.sql.types.Decimal.set(Decimal.scala:140) at org.apache.spark.sql.types.Decimal$.apply(Decimal.scala:434) at org.apache.spark.sql.types.Decimal.apply(Decimal.scala) at org.apache.spark.sql.catalyst.expressions.GeneratedClass$SpecificUnsafeProjection.apply(Unknown Source) at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder.toRow(ExpressionEncoder.scala:287) ... 59 more ``` ## How was this patch tested? Add new test suite into `DecimalSuite` Author: Kazuaki Ishizaki <[email protected]> Closes #17684 from kiszk/SPARK-20341. (cherry picked from commit a750a59) Signed-off-by: Wenchen Fan <[email protected]>

cloud-fan · 2017-04-21T14:30:41Z

it conflicts with branch 2.1, can you send a new PR? thanks!

…alue range ## What changes were proposed in this pull request? This PR avoids an exception in the case where `scala.math.BigInt` has a value that does not fit into long value range (e.g. `Long.MAX_VALUE+1`). When we run the following code by using the current Spark, the following exception is thrown. This PR keeps the value using `BigDecimal` if we detect such an overflow case by catching `ArithmeticException`. Sample program: ``` case class BigIntWrapper(value:scala.math.BigInt)``` spark.createDataset(BigIntWrapper(scala.math.BigInt("10000000000000000002"))::Nil).show ``` Exception: ``` Error while encoding: java.lang.ArithmeticException: BigInteger out of long range staticinvoke(class org.apache.spark.sql.types.Decimal$, DecimalType(38,0), apply, assertnotnull(assertnotnull(input[0, org.apache.spark.sql.BigIntWrapper, true])).value, true) AS value#0 java.lang.RuntimeException: Error while encoding: java.lang.ArithmeticException: BigInteger out of long range staticinvoke(class org.apache.spark.sql.types.Decimal$, DecimalType(38,0), apply, assertnotnull(assertnotnull(input[0, org.apache.spark.sql.BigIntWrapper, true])).value, true) AS value#0 at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder.toRow(ExpressionEncoder.scala:290) at org.apache.spark.sql.SparkSession$$anonfun$2.apply(SparkSession.scala:454) at org.apache.spark.sql.SparkSession$$anonfun$2.apply(SparkSession.scala:454) at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234) at scala.collection.immutable.List.foreach(List.scala:381) at scala.collection.TraversableLike$class.map(TraversableLike.scala:234) at scala.collection.immutable.List.map(List.scala:285) at org.apache.spark.sql.SparkSession.createDataset(SparkSession.scala:454) at org.apache.spark.sql.Agg$$anonfun$18.apply$mcV$sp(MySuite.scala:192) at org.apache.spark.sql.Agg$$anonfun$18.apply(MySuite.scala:192) at org.apache.spark.sql.Agg$$anonfun$18.apply(MySuite.scala:192) at org.scalatest.Transformer$$anonfun$apply$1.apply$mcV$sp(Transformer.scala:22) at org.scalatest.OutcomeOf$class.outcomeOf(OutcomeOf.scala:85) at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104) at org.scalatest.Transformer.apply(Transformer.scala:22) at org.scalatest.Transformer.apply(Transformer.scala:20) at org.scalatest.FunSuiteLike$$anon$1.apply(FunSuiteLike.scala:166) at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:68) at org.scalatest.FunSuiteLike$class.invokeWithFixture$1(FunSuiteLike.scala:163) at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:175) at org.scalatest.FunSuiteLike$$anonfun$runTest$1.apply(FunSuiteLike.scala:175) at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306) at org.scalatest.FunSuiteLike$class.runTest(FunSuiteLike.scala:175) ... Caused by: java.lang.ArithmeticException: BigInteger out of long range at java.math.BigInteger.longValueExact(BigInteger.java:4531) at org.apache.spark.sql.types.Decimal.set(Decimal.scala:140) at org.apache.spark.sql.types.Decimal$.apply(Decimal.scala:434) at org.apache.spark.sql.types.Decimal.apply(Decimal.scala) at org.apache.spark.sql.catalyst.expressions.GeneratedClass$SpecificUnsafeProjection.apply(Unknown Source) at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder.toRow(ExpressionEncoder.scala:287) ... 59 more ``` ## How was this patch tested? Add new test suite into `DecimalSuite` Author: Kazuaki Ishizaki <[email protected]> Closes apache#17684 from kiszk/SPARK-20341.

initial commit

9a15ba2

gatorsmile reviewed Apr 19, 2017

View reviewed changes

address review comment

655e115

gatorsmile reviewed Apr 21, 2017

View reviewed changes

address review comment

6425239

asfgit closed this in a750a59 Apr 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-20341][SQL] Support BigInt's value that does not fit in long value range #17684

[SPARK-20341][SQL] Support BigInt's value that does not fit in long value range #17684

kiszk commented Apr 19, 2017

SparkQA commented Apr 19, 2017

gatorsmile left a comment

gatorsmile Apr 19, 2017

gatorsmile Apr 19, 2017

kiszk commented Apr 19, 2017 •

edited

Loading

gatorsmile commented Apr 20, 2017

SparkQA commented Apr 20, 2017

kiszk commented Apr 20, 2017

gatorsmile Apr 21, 2017

kiszk Apr 21, 2017

gatorsmile commented Apr 21, 2017

SparkQA commented Apr 21, 2017

cloud-fan commented Apr 21, 2017

cloud-fan commented Apr 21, 2017

		@@ -135,11 +135,17 @@ final class Decimal extends Ordered[Decimal] with Serializable {
		* Set this Decimal to the given BigInteger value. Will have precision 38 and scale 0.

[SPARK-20341][SQL] Support BigInt's value that does not fit in long value range #17684

[SPARK-20341][SQL] Support BigInt's value that does not fit in long value range #17684

Conversation

kiszk commented Apr 19, 2017

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Apr 19, 2017

gatorsmile left a comment

Choose a reason for hiding this comment

gatorsmile Apr 19, 2017

Choose a reason for hiding this comment

gatorsmile Apr 19, 2017

Choose a reason for hiding this comment

kiszk commented Apr 19, 2017 • edited Loading

gatorsmile commented Apr 20, 2017

SparkQA commented Apr 20, 2017

kiszk commented Apr 20, 2017

gatorsmile Apr 21, 2017

Choose a reason for hiding this comment

kiszk Apr 21, 2017

Choose a reason for hiding this comment

gatorsmile commented Apr 21, 2017

SparkQA commented Apr 21, 2017

cloud-fan commented Apr 21, 2017

cloud-fan commented Apr 21, 2017

kiszk commented Apr 19, 2017 •

edited

Loading