[SPARK-19727][SQL][followup] Fix for round function that modifies original column #19576
Conversation
| precision: Int, | ||
| scale: Int, | ||
| roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Option[Decimal] = { | ||
| roundMode: BigDecimal.RoundingMode.Value = ROUND_HALF_UP): Decimal = { |
There was a problem hiding this comment.
Option is hard to use in java code(the codegen path), so I change the return type to nullable Decimal.
|
Test build #83059 has finished for PR 19576 at commit
|
| /** | ||
| * Create new `Decimal` with precision and scale given in `decimalType` (if any), | ||
| * returning null if it overflows or creating a new `value` and returning it if successful. | ||
| * |
There was a problem hiding this comment.
Nit: remove this useless line.
| ) | ||
| } | ||
|
|
||
| test("round/bround with table columns") { |
There was a problem hiding this comment.
This is an end-to-end test for code-gen path. Could we add a unit test case in MathExpressionsSuite?
There was a problem hiding this comment.
All the tests in MathExpressionsSuite are testing about literals, I think it's better to improve MathExpressionsSuite for attributes in a new PR, instead of doing it in a folllow-up PR. BTW the original PR didn't add test in MathExpressionsSuite either.
|
LGTM |
|
Test build #83176 has finished for PR 19576 at commit
|
|
Thanks! Merged to master/2.2 |
…ginal column ## What changes were proposed in this pull request? This is a followup of #17075 , to fix the bug in codegen path. ## How was this patch tested? new regression test Author: Wenchen Fan <[email protected]> Closes #19576 from cloud-fan/bug. (cherry picked from commit 7fdacbc) Signed-off-by: gatorsmile <[email protected]>
…ginal column ## What changes were proposed in this pull request? This is a followup of apache#17075 , to fix the bug in codegen path. ## How was this patch tested? new regression test Author: Wenchen Fan <[email protected]> Closes apache#19576 from cloud-fan/bug. (cherry picked from commit 7fdacbc) Signed-off-by: gatorsmile <[email protected]>
3 participants
What changes were proposed in this pull request?
This is a followup of #17075 , to fix the bug in codegen path.
How was this patch tested?
new regression test