Commit d17f687
authored
fix(arrow/compute): accept type max value in safe decimal-to-int cast (#862)
### 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 used `value >= max`, where `max` is the
*inclusive* maximum of the output integer type (e.g. `math.MaxInt64` for
`int64`, computed via `MaxOf[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`/`int64` and the unsigned
variants), not just `int64`.
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?
- In `decimalToIntImpl`
(`arrow/compute/internal/kernels/numeric_cast.go`), change the
upper-bound overflow check from `value >= max` to `value > max` so the
inclusive maximum is accepted, matching the already-inclusive
lower-bound check (`value < min`). The `decimal[T]` helper interface now
requires `Greater` instead of `GreaterEqual`; both `decimal128.Num` and
`decimal256.Num` already implement it.
### Are these changes tested?
Yes. Added an `int64 bounds inclusive` regression subtest to
`TestDecimal128ToInt` and `TestDecimal256ToInt` that assert, with
`AllowIntOverflow=false`:
- `math.MaxInt64` (`9223372036854775807`) and `math.MinInt64`
(`-9223372036854775808`) cast successfully, and
- values one beyond either bound still fail as overflow.
`go test ./arrow/compute/...` passes and `go vet` is 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 an `ErrInvalid` "integer value out of
bounds" error. Values genuinely outside the range still error as before.1 parent 4a540f5 commit d17f687
2 files changed
Lines changed: 37 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
721 | 721 | | |
722 | 722 | | |
723 | 723 | | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
724 | 740 | | |
725 | 741 | | |
726 | 742 | | |
| |||
828 | 844 | | |
829 | 845 | | |
830 | 846 | | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
| 862 | + | |
831 | 863 | | |
832 | 864 | | |
833 | 865 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
75 | | - | |
| 75 | + | |
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | | - | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
81 | 84 | | |
82 | 85 | | |
83 | 86 | | |
| |||
0 commit comments