fix: align wide-decimal ANSI overflow value with Spark (#5211) - #5276
Open
sam-1112 wants to merge 4 commits into
Open
fix: align wide-decimal ANSI overflow value with Spark (#5211)#5276sam-1112 wants to merge 4 commits into
sam-1112 wants to merge 4 commits into
Conversation
Member
|
should we change this to draft? as #5169 hasnt been merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #5211.
Rationale for this change
This is a follow-up to #5072 and #5169.
Under ANSI mode, wide-decimal overflow raises
NUMERIC_VALUE_OUT_OF_RANGE, but thevaluereported by Comet did not match Spark.Spark reports the pre-
toPrecisionintermediate value usingtoPlainString. Comet instead formatted the already-rescaledi256value at the output scale, which introduced a spurious all zero fractional suffix, such as.000000.For multiplication, Spark also applies
MathContext(39, DOWN)before formatting the reported value. Without this step, values with high storage scales, such asDecimal(38, 18), retained a long.000…suffix that Spark does not emit.What changes are included in this PR?
MathContext(39, DOWN)when constructing the reportedvalue.NumericValueOutOfRangeparameters usingjava.math.BigDecimalshims so that ScalaBigDecimal's defaultDECIMAL128MathContextdoes not rewrite large integer values.CometExpressionSuiteto verify that thevalueparameter matches Spark and does not contain a spurious.000000suffix.How are these changes tested?
wide_decimal_binary_expr.rscovering overflow-value formatting for addition and multiplication, including theDecimal(38, 18)multiplication case.CometExpressionSuitefor both legacy and ANSI modes.