Skip to content

Commit

Permalink
[SPARK-26233][SQL] CheckOverflow when encoding a decimal value
Browse files Browse the repository at this point in the history
## What changes were proposed in this pull request?

When we encode a Decimal from external source we don't check for overflow. That method is useful not only in order to enforce that we can represent the correct value in the specified range, but it also changes the underlying data to the right precision/scale. Since in our code generation we assume that a decimal has exactly the same precision and scale of its data type, missing to enforce it can lead to corrupted output/results when there are subsequent transformations.

## How was this patch tested?

added UT

Closes #23210 from mgaido91/SPARK-26233.

Authored-by: Marco Gaido <marcogaido91@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
  • Loading branch information
mgaido91 authored and dongjoon-hyun committed Dec 4, 2018
1 parent f982ca0 commit 556d83e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
Expand Up @@ -106,12 +106,12 @@ object RowEncoder {
returnNullable = false)

case d: DecimalType =>
StaticInvoke(
CheckOverflow(StaticInvoke(
Decimal.getClass,
d,
"fromDecimal",
inputObject :: Nil,
returnNullable = false)
returnNullable = false), d)

case StringType =>
StaticInvoke(
Expand Down
Expand Up @@ -1647,6 +1647,15 @@ class DatasetSuite extends QueryTest with SharedSQLContext {
checkDataset(ds, data: _*)
checkAnswer(ds.select("x"), Seq(Row(1), Row(2)))
}

test("SPARK-26233: serializer should enforce decimal precision and scale") {
val s = StructType(Seq(StructField("a", StringType), StructField("b", DecimalType(38, 8))))
val encoder = RowEncoder(s)
implicit val uEnc = encoder
val df = spark.range(2).map(l => Row(l.toString, BigDecimal.valueOf(l + 0.1111)))
checkAnswer(df.groupBy(col("a")).agg(first(col("b"))),
Seq(Row("0", BigDecimal.valueOf(0.1111)), Row("1", BigDecimal.valueOf(1.1111))))
}
}

case class TestDataUnion(x: Int, y: Int, z: Int)
Expand Down

0 comments on commit 556d83e

Please sign in to comment.