[SPARK-20211][SQL] Fix the Precision and Scale of Decimal Values when the Input is BigDecimal between -1.0 and 1.0 by gatorsmile · Pull Request #18244 · apache/spark

gatorsmile · 2017-06-08T23:37:09Z

What changes were proposed in this pull request?

The precision and scale of decimal values are wrong when the input is BigDecimal between -1.0 and 1.0.

The BigDecimal's precision is the digit count starts from the leftmost nonzero digit based on the JAVA's BigDecimal definition. However, our Decimal decision follows the database decimal standard, which is the total number of digits, including both to the left and the right of the decimal point. Thus, this PR is to fix the issue by doing the conversion.

Before this PR, the following queries failed:

select1>0.0001select floor(0.0001)
select ceil(0.0001)

How was this patch tested?

Added test cases.

gatorsmile · 2017-06-08T23:38:51Z

cc @cloud-fan @davies

cloud-fan · 2017-06-09T01:01:58Z

LGTM, pending test

SparkQA · 2017-06-09T01:53:17Z

Test build #77826 has finished for PR 18244 at commit a7396be.

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

SparkQA · 2017-06-09T02:09:15Z

Test build #77828 has finished for PR 18244 at commit 8e654bc.

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

davies · 2017-06-09T05:14:18Z

 this.decimalVal = decimal
 this.longVal = 0L
- this._precision = decimal.precision
+ if (decimal.compare(BigDecimal(1.0)) == -1 && decimal.compare(BigDecimal(-1.0)) == 1) {


if (decimal.presision <= decimal.scale) {
}

just if (decimal.presision < decimal.scale) {
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala#L387

Thank you! @davies

@stanzhai That is not enough. 0.90's precision is 2 and scale is also 2. When the scale and precision are identical, we still need to update the precision.

Don't we allow the case that precision equals to scale? Seems it works for the example expressions.
scala> sql("select floor(0.9)").show() +----------+ |FLOOR(0.9)| +----------+ | 0| +----------+ scala> sql("select ceil(0.9)").show() +---------+ |CEIL(0.9)| +---------+ | 1| +---------+ scala> sql("select 1 > 0.9").show() +-----------------------+ |(CAST(1 AS BIGINT) > 0)| +-----------------------+ | true| +-----------------------+

We need to correct the precision to make it consistent with what the other database system.

Do you mean 0.9 has precision 2 and scale 1 in other database systems? I just tried MySQL, it seems to be precision=1 and scale=1. Not sure if other database systems have the behavior as you said.

Btw, seems that the numbers like 0.9 are the only cases that the precision equals to the scale. If we force precision as scale + 1, looks like we don't support precision=scale anymore for DecimalType. Then we may need to update the related check and the document in DecimalType which explicitly says that scale can be equal to precision. Maybe I miss anything about this?

You can try DB2. 0.9's precision is 2, scale is 1. 0.90's precision is 3, scale is 2

We should still allow the case in which the precision equals to the scale. In the traditional database, .99's precision is 2 and scale is 2; but 0.99's precision is 3 and scale is 2

SparkQA · 2017-06-09T06:24:17Z

Test build #77838 has started for PR 18244 at commit d36be12.

davies · 2017-06-09T06:26:32Z

lgtm

stanzhai · 2017-06-09T06:44:11Z

 this.decimalVal = decimal
 this.longVal = 0L
- this._precision = decimal.precision
+ if (decimal.precision <= decimal.scale) {


But the comment is // For Decimal, we expect the precision is equal to or large than the scale.
= has been processed within the function floor and ceil.
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala#L387
This is reason that I think we should use if (decimal.precision < decimal.scale), and it works fine for 0.90.

This fix is not for floor and ceil only. We need to correct the precision if the value is from BigDecimal.

Got it, thanks for the fix!

discipleforteen · 2017-06-09T07:12:55Z

how about
select 1 > 0.0001D
select floor(0.0001D)
select ceil(0.0001D)

i think constant doube value should be parsed to double as old spark version, not to bigdecimal in antlr4

gatorsmile · 2017-06-09T17:40:18Z

retest this please

gatorsmile · 2017-06-09T17:48:22Z

@discipleforteen Your question is not related to this PR. Converting to the decimal is pretty common for avoiding loss of precision. For example, the link shows how DB2 choose the type when users specify the constant.

SparkQA · 2017-06-09T20:13:10Z

Test build #77848 has finished for PR 18244 at commit d36be12.

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

viirya · 2017-06-10T03:01:32Z

LGTM

gatorsmile · 2017-06-10T17:30:10Z

Thanks! Merging to master.

Will submit PRs for 2.2/2.1/2.0

…al Values when the Input is BigDecimal between -1.0 and 1.0 ### What changes were proposed in this pull request? This PR is to backport #18244 to 2.2 --- The precision and scale of decimal values are wrong when the input is BigDecimal between -1.0 and 1.0. The BigDecimal's precision is the digit count starts from the leftmost nonzero digit based on the [JAVA's BigDecimal definition](https://docs.oracle.com/javase/7/docs/api/java/math/BigDecimal.html). However, our Decimal decision follows the database decimal standard, which is the total number of digits, including both to the left and the right of the decimal point. Thus, this PR is to fix the issue by doing the conversion. Before this PR, the following queries failed: ```SQL select 1 > 0.0001 select floor(0.0001) select ceil(0.0001) ``` ### How was this patch tested? Added test cases. Author: gatorsmile <gatorsmile@gmail.com> Closes#18297 from gatorsmile/backport18244.

…al Values when the Input is BigDecimal between -1.0 and 1.0 ### What changes were proposed in this pull request? This PR is to backport #18244 to 2.2 --- The precision and scale of decimal values are wrong when the input is BigDecimal between -1.0 and 1.0. The BigDecimal's precision is the digit count starts from the leftmost nonzero digit based on the [JAVA's BigDecimal definition](https://docs.oracle.com/javase/7/docs/api/java/math/BigDecimal.html). However, our Decimal decision follows the database decimal standard, which is the total number of digits, including both to the left and the right of the decimal point. Thus, this PR is to fix the issue by doing the conversion. Before this PR, the following queries failed: ```SQL select 1 > 0.0001 select floor(0.0001) select ceil(0.0001) ``` ### How was this patch tested? Added test cases. Author: gatorsmile <gatorsmile@gmail.com> Closes#18297 from gatorsmile/backport18244. (cherry picked from commit 6265119) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

… the Input is BigDecimal between -1.0 and 1.0 ### What changes were proposed in this pull request? The precision and scale of decimal values are wrong when the input is BigDecimal between -1.0 and 1.0. The BigDecimal's precision is the digit count starts from the leftmost nonzero digit based on the [JAVA's BigDecimal definition](https://docs.oracle.com/javase/7/docs/api/java/math/BigDecimal.html). However, our Decimal decision follows the database decimal standard, which is the total number of digits, including both to the left and the right of the decimal point. Thus, this PR is to fix the issue by doing the conversion. Before this PR, the following queries failed: ```SQL select 1 > 0.0001 select floor(0.0001) select ceil(0.0001) ``` ### How was this patch tested? Added test cases. Author: Xiao Li <gatorsmile@gmail.com> Closesapache#18244 from gatorsmile/bigdecimal.

…al Values when the Input is BigDecimal between -1.0 and 1.0 This PR is to backport apache#18244 to 2.2 --- The precision and scale of decimal values are wrong when the input is BigDecimal between -1.0 and 1.0. The BigDecimal's precision is the digit count starts from the leftmost nonzero digit based on the [JAVA's BigDecimal definition](https://docs.oracle.com/javase/7/docs/api/java/math/BigDecimal.html). However, our Decimal decision follows the database decimal standard, which is the total number of digits, including both to the left and the right of the decimal point. Thus, this PR is to fix the issue by doing the conversion. Before this PR, the following queries failed: ```SQL select 1 > 0.0001 select floor(0.0001) select ceil(0.0001) ``` Added test cases. Author: gatorsmile <gatorsmile@gmail.com> Closesapache#18297 from gatorsmile/backport18244. (cherry picked from commit 6265119) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

fix.
a7396be

gatorsmile mentioned this pull request Jun 8, 2017
[SPARK-20211][SQL]Fix a bug in FLOOR and CEIL when a decimal's precision < scale #17529
Closed

fix.
8e654bc

davies reviewed Jun 9, 2017
View reviewed changes

fix.
d36be12

stanzhai reviewed Jun 9, 2017
View reviewed changes

asfgit closed this in 8e96acfJun 10, 2017

gatorsmile mentioned this pull request Jun 14, 2017
[SPARK-20211][SQL][Backport-2.2] Fix the Precision and Scale of Decimal Values when the Input is BigDecimal between -1.0 and 1.0 #18297
Closed

Uh oh!

Conversation

gatorsmile commented Jun 8, 2017

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

gatorsmile commented Jun 8, 2017

Uh oh!

cloud-fan commented Jun 9, 2017

Uh oh!

SparkQA commented Jun 9, 2017

Uh oh!

SparkQA commented Jun 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viiryaJun 9, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 9, 2017

Uh oh!

davies commented Jun 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

discipleforteen commented Jun 9, 2017

Uh oh!

gatorsmile commented Jun 9, 2017

Uh oh!

gatorsmile commented Jun 9, 2017

Uh oh!

SparkQA commented Jun 9, 2017

Uh oh!

viirya commented Jun 10, 2017

Uh oh!

gatorsmile commented Jun 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

viiryaJun 9, 2017•
edited
Loading