advitrocks9 opened a new pull request, #52469: URL: https://github.com/apache/arrow/pull/52469
### Rationale for this change With `allow_float_truncate=true` and `allow_int_overflow=false`, a float to int cast never checks the range, so out of range values come back as garbage instead of an error. Casting a float64 array to int32 on x86-64: ``` 1e308 -> -2147483648 3e9 -> -2147483648 -3e9 -> -2147483648 NaN -> -2147483648 ``` The same values with an int64 input fail with `Integer value 3000000000 not in range`. ### What changes are included in this PR? `CastFloatingToInteger` runs the unchecked conversion and then only checks `allow_float_truncate`. In the default safe cast that check also rejects out of range values, because they don't survive the round trip back to float, but once truncation is allowed nothing reads `allow_int_overflow` at all. I added a `WasOutOfRange` checker next to `WasTruncated` and made the existing block loop take the checker as a template parameter, so the new check runs through the same fast path. It only looks at the input. The bounds are compared as powers of two because `static_cast<double>(INT64_MAX)` rounds up to 2^63, and the lower bound is compared after `std::trunc` so that -128.5 still casts to int8 when truncation is allowed. NaN and infinities are out of range. The safe cast path is unchanged, except that out of range values are now reported as `out of range` rather than `was truncated`. In R, `%/%` on integers builds `if_else(y == 0, NA, cast(floor(x / y)))`. `if_else` evaluates the cast for every row, including the Inf rows it then masks, so that cast now also passes `allow_int_overflow = TRUE` to keep `x %/% 0L` returning NA. ### Are these changes tested? Yes, `Cast.FloatingToIntOverflow` covers int8, uint8 and the 64-bit edges for float16, float32 and float64, with and without truncation. Without the kernel change it fails 25 of its assertions. `arrow-compute-scalar-cast-test` passes under ASAN and UBSAN. I didn't have an R toolchain locally, so the R change is checked against the C++ cast it calls: `cast([inf], int32, allow_float_truncate=true)` now errors, and passes again with `allow_int_overflow=true`. ### Are there any user-facing changes? Yes. Casts with `allow_float_truncate=true` and `allow_int_overflow=false` now raise on out of range, NaN and infinite values. This includes R's `as.integer()` and `as.integer64()` in dplyr queries, which set `allow_float_truncate = TRUE`. The error message for out of range values in safe casts changes from `was truncated` to `out of range`. **This PR contains a "Critical Fix".** These casts silently produced incorrect values. * GitHub Issue: #46557 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
