-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-26038] Decimal toScalaBigInt/toJavaBigInteger for decimals not fitting in long #23022
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
cc @hvanhovell |
hvanhovell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - pending jenkins
|
Test build #98772 has finished for PR 23022 at commit
|
| assert(decimal.toJavaBigDecimal.unscaledValue.toString === "9223372036854775808") | ||
| } | ||
|
|
||
| test("SPARK-26038: toScalaBigInt/toJavaBigInteger not fitting long") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we test the branch when decimalVal.ne(null) is false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just re-run DecimalSuite in the debugger, and haven't hit breakpoints in the branches. It seems there are no tests for nulls at all.
|
Test build #98781 has finished for PR 23022 at commit
|
|
LGTM |
|
ping @hvanhovell |
hvanhovell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Merging to master. Thank! |
… fitting in long ## What changes were proposed in this pull request? Fix Decimal `toScalaBigInt` and `toJavaBigInteger` used to only work for decimals not fitting long. ## How was this patch tested? Added test to DecimalSuite. Closes apache#23022 from juliuszsompolski/SPARK-26038. Authored-by: Juliusz Sompolski <[email protected]> Signed-off-by: Herman van Hovell <[email protected]>
… fitting in long ## What changes were proposed in this pull request? Fix Decimal `toScalaBigInt` and `toJavaBigInteger` used to only work for decimals not fitting long. ## How was this patch tested? Added test to DecimalSuite. Closes apache#23022 from juliuszsompolski/SPARK-26038. Authored-by: Juliusz Sompolski <[email protected]> Signed-off-by: Herman van Hovell <[email protected]>
…decimals not fitting in long This is a Spark 2.4.x backport of #23022. Original description follows below: ## What changes were proposed in this pull request? Fix Decimal `toScalaBigInt` and `toJavaBigInteger` used to only work for decimals not fitting long. ## How was this patch tested? Added test to DecimalSuite. Closes #24928 from JoshRosen/joshrosen/SPARK-26038-backport. Authored-by: Juliusz Sompolski <[email protected]> Signed-off-by: Josh Rosen <[email protected]>
…decimals not fitting in long This is a Spark 2.4.x backport of apache#23022. Original description follows below: ## What changes were proposed in this pull request? Fix Decimal `toScalaBigInt` and `toJavaBigInteger` used to only work for decimals not fitting long. ## How was this patch tested? Added test to DecimalSuite. Closes apache#24928 from JoshRosen/joshrosen/SPARK-26038-backport. Authored-by: Juliusz Sompolski <[email protected]> Signed-off-by: Josh Rosen <[email protected]>
…decimals not fitting in long This is a Spark 2.4.x backport of apache#23022. Original description follows below: ## What changes were proposed in this pull request? Fix Decimal `toScalaBigInt` and `toJavaBigInteger` used to only work for decimals not fitting long. ## How was this patch tested? Added test to DecimalSuite. Closes apache#24928 from JoshRosen/joshrosen/SPARK-26038-backport. Authored-by: Juliusz Sompolski <[email protected]> Signed-off-by: Josh Rosen <[email protected]>
What changes were proposed in this pull request?
Fix Decimal
toScalaBigIntandtoJavaBigIntegerused to only work for decimals not fitting long.How was this patch tested?
Added test to DecimalSuite.