fix(arrow-cast): do not truncate integers when casting to Decimal32/64 - #10707
Open
yongster wants to merge 1 commit into
Open
fix(arrow-cast): do not truncate integers when casting to Decimal32/64#10707yongster wants to merge 1 commit into
yongster wants to merge 1 commit into
Conversation
Closes apache#10706. cast_integer_to_decimal used AsPrimitive before the precision check. That silently wraps values that do not fit the decimal native type, so Int64(5_000_000_000) became Decimal32 705032704, and UInt32(4e9) / UInt64::MAX could become negative. Convert through a range-checked i128 path first, then apply scale and precision. Safe casts become null; unsafe casts error with the original value. Add regression tests for Int64 -> Decimal32, UInt32 -> Decimal32, and UInt64::MAX -> Decimal64.
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?
Rationale for this change
cast/cast_with_optionsfrom an integer array toDecimal32orDecimal64can rewrite the value instead of rejecting it.cast_integer_to_decimalfirst usedAsPrimitive(as), which wraps whenthe source integer does not fit the decimal native type. The precision check
then ran on the already-truncated value. If that wrapped value happened to
fit the requested precision, it was stored as if it were the original number.
Examples on current
main:Int64(5_000_000_000) -> Decimal32(9, 0)(safe: true) becomes705032704UInt32(4_000_000_000) -> Decimal32(9, 0)(safe: false) becomes-294967296UInt64::MAX -> Decimal64(18, 0)(safe: false) becomes-1The same inputs cast to
Decimal128already return null /Err, becausei64 as i128is lossless.What changes are included in this PR?
i128path(
num_cast+DecimalCast) before applying scale and precision.safe: truewrites null when the original value does not fit;safe: falsereturns
Errand reports the original value.Int64 -> Decimal32,UInt32 -> Decimal32, andUInt64::MAX -> Decimal64.Are these changes tested?
Yes. I ran: