fix: do not overflow in unwrap_cast for decimals with a negative scale - #24897
Open
edubraqd wants to merge 1 commit into
Open
fix: do not overflow in unwrap_cast for decimals with a negative scale#24897edubraqd wants to merge 1 commit into
unwrap_cast for decimals with a negative scale#24897edubraqd wants to merge 1 commit into
Conversation
`try_cast_numeric_literal` computes `10_i128.pow(scale as u32)` for both the
literal's and the target's decimal scale. A negative scale wraps to a huge
exponent, so simplifying an expression such as
arrow_cast(1, 'Decimal128(10, -2)') = 100
panicked with "attempt to multiply with overflow" in the `unwrap_cast`
simplifier.
A negative scale means the decimal holds multiples of `10^-scale`, so the
integer rescaling this path relies on does not apply. Return `None` for such
casts so the simplifier leaves them in place.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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?
unwrap_cast#24896.Rationale for this change
try_cast_numeric_literalcomputes10_i128.pow(scale as u32)for both the literal's and the target's decimal scale. A negative scale wraps to a huge exponent, so simplifyingarrow_cast(1, 'Decimal128(10, -2)') = 100panicked with "attempt to multiply with overflow" in theunwrap_castsimplifier.What changes are included in this PR?
A negative scale means the decimal holds multiples of
10^-scale, so the integer rescaling this path relies on does not apply.try_cast_numeric_literalnow returnsNonewhen either side has a negative scale, so the simplifier leaves the cast in place, the same way it already does for other casts it cannot express.Are these changes tested?
Yes.
test_try_cast_to_type_negative_scale_decimalcovers a negative-scale target forDecimal32/Decimal64/Decimal128and a negative-scale literal; each used to panic.Are there any user-facing changes?
No panic. Such comparisons are no longer simplified by
unwrap_cast; they were never simplified correctly before, since the process panicked.