fix(arrow/compute): accept type max value in safe decimal-to-int cast#862
Merged
Merged
Conversation
The safe decimal->integer cast kernel checked `value >= max`, where max is the inclusive maximum of the output integer type (e.g. math.MaxInt64). This rejected the maximum value itself as out of bounds for every integer output type, even though it is a valid in-range value. Use a strict `value > max` check so the inclusive upper bound is accepted, matching the already inclusive lower bound. Adds boundary regression tests covering math.MinInt64/math.MaxInt64 for both decimal128 and decimal256 inputs.
lidavidm
approved these changes
Jun 23, 2026
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.
Rationale for this change
The safe
decimal128/decimal256→ integer cast kernel range-checks each value against the output type's bounds before truncating to the low 64 bits. The upper-bound check usedvalue >= max, wheremaxis the inclusive maximum of the output integer type (e.g.math.MaxInt64forint64, computed viaMaxOf[T]()). As a result a value exactly equal to the type maximum was incorrectly rejected as "integer value out of bounds", even though it is a valid in-range value. This affects every integer output type (int8/int16/int32/int64and the unsigned variants), not justint64.This was noticed while working around the behavior downstream in the ADBC Snowflake driver (adbc-drivers/snowflake#161), where it was suggested to fix it upstream here.
What changes are included in this PR?
decimalToIntImpl(arrow/compute/internal/kernels/numeric_cast.go), change the upper-bound overflow check fromvalue >= maxtovalue > maxso the inclusive maximum is accepted, matching the already-inclusive lower-bound check (value < min). Thedecimal[T]helper interface now requiresGreaterinstead ofGreaterEqual; bothdecimal128.Numanddecimal256.Numalready implement it.Are these changes tested?
Yes. Added an
int64 bounds inclusiveregression subtest toTestDecimal128ToIntandTestDecimal256ToIntthat assert, withAllowIntOverflow=false:math.MaxInt64(9223372036854775807) andmath.MinInt64(-9223372036854775808) cast successfully, andgo test ./arrow/compute/...passes andgo vetis clean.Are there any user-facing changes?
Yes — a bug fix. A safe (non-
AllowIntOverflow) decimal→integer cast of a value equal to the destination type's maximum (e.g.math.MaxInt64) now succeeds instead of returning anErrInvalid"integer value out of bounds" error. Values genuinely outside the range still error as before.