This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 5746b443f7 fix: prevent time64us and time64ns from wrapping
out-of-bounds seconds to valid times (#11899)
5746b443f7 is described below
commit 5746b443f70b4e55676be59cea317501a94ec810
Author: Ryosuke Tanaka <[email protected]>
AuthorDate: Tue Oct 27 15:27:24 2026 +0000
fix: prevent time64us and time64ns from wrapping out-of-bounds seconds to
valid times (#11899)
# Which issue does this PR close?
Closes #11893.
# Rationale for this change
In `arrow-array/src/temporal_conversions.rs`, `time64us_to_time` and
`time64ns_to_time` (as well as `time32s_to_time` and `time32ms_to_time`)
converted seconds using `as u32`. For large `Time64` inputs (e.g. `(2^32
+ 5) * 1_000_000_000` ns), the high 32 bits of seconds were truncated,
causing values far outside one day to wrap around into valid times
within a day (e.g. `00:00:05`) instead of returning `None`.
# What changes are included in this PR?
- Replaced `as u32` conversions in `time32s_to_time`,
`time32ms_to_time`, `time64us_to_time`, and `time64ns_to_time` with
`u32::try_from(..).ok()?`.
- Added unit tests asserting that out-of-bounds / wrapped values for
`Time32` and `Time64` consistently return `None` while valid values
continue to convert as expected.
# Are these changes tested?
Yes, new unit tests covering out-of-bounds, negative, extreme
(`i64::MAX`/`i64::MIN`), and wrapped inputs have been added to
`temporal_conversions::tests`. Downstream tests in `arrow-cast` and
`arrow-arith` continue to pass.
# Are there any user-facing changes?
Out-of-bounds `Time64` values exceeding 2^32 seconds now return `None`
rather than wrapping around to valid timestamps.
Co-authored-by: Ryosuke Tanaka
<[email protected]>
---
arrow-array/src/temporal_conversions.rs | 37 ++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/arrow-array/src/temporal_conversions.rs
b/arrow-array/src/temporal_conversions.rs
index e264a15bdb..328514b2f1 100644
--- a/arrow-array/src/temporal_conversions.rs
+++ b/arrow-array/src/temporal_conversions.rs
@@ -74,7 +74,7 @@ pub fn date64_to_datetime(v: i64) -> Option<NaiveDateTime> {
/// converts a `i32` representing a `time32(s)` to [`NaiveDateTime`]
#[inline]
pub fn time32s_to_time(v: i32) -> Option<NaiveTime> {
- NaiveTime::from_num_seconds_from_midnight_opt(v as u32, 0)
+ NaiveTime::from_num_seconds_from_midnight_opt(u32::try_from(v).ok()?, 0)
}
/// converts a `i32` representing a `time32(ms)` to [`NaiveDateTime`]
@@ -83,10 +83,10 @@ pub fn time32ms_to_time(v: i32) -> Option<NaiveTime> {
let v = v as i64;
NaiveTime::from_num_seconds_from_midnight_opt(
// extract seconds from milliseconds
- (v / MILLISECONDS) as u32,
+ u32::try_from(v / MILLISECONDS).ok()?,
// discard extracted seconds and convert milliseconds to
// nanoseconds
- (v % MILLISECONDS * MICROSECONDS) as u32,
+ u32::try_from(v % MILLISECONDS * MICROSECONDS).ok()?,
)
}
@@ -95,10 +95,10 @@ pub fn time32ms_to_time(v: i32) -> Option<NaiveTime> {
pub fn time64us_to_time(v: i64) -> Option<NaiveTime> {
NaiveTime::from_num_seconds_from_midnight_opt(
// extract seconds from microseconds
- (v / MICROSECONDS) as u32,
+ u32::try_from(v / MICROSECONDS).ok()?,
// discard extracted seconds and convert microseconds to
// nanoseconds
- (v % MICROSECONDS * MILLISECONDS) as u32,
+ u32::try_from(v % MICROSECONDS * MILLISECONDS).ok()?,
)
}
@@ -107,9 +107,9 @@ pub fn time64us_to_time(v: i64) -> Option<NaiveTime> {
pub fn time64ns_to_time(v: i64) -> Option<NaiveTime> {
NaiveTime::from_num_seconds_from_midnight_opt(
// extract seconds from nanoseconds
- (v / NANOSECONDS) as u32,
+ u32::try_from(v / NANOSECONDS).ok()?,
// discard extracted seconds
- (v % NANOSECONDS) as u32,
+ u32::try_from(v % NANOSECONDS).ok()?,
)
}
@@ -348,9 +348,10 @@ pub fn as_duration<T: ArrowPrimitiveType>(v: i64) ->
Option<Duration> {
#[cfg(test)]
mod tests {
use crate::temporal_conversions::{
- NANOSECONDS, date64_to_datetime, split_second,
timestamp_ms_to_datetime,
- timestamp_ns_to_datetime, timestamp_s_to_date, timestamp_s_to_datetime,
- timestamp_s_to_time, timestamp_us_to_datetime,
+ NANOSECONDS, date64_to_datetime, split_second, time32ms_to_time,
time32s_to_time,
+ time64ns_to_time, time64us_to_time, timestamp_ms_to_datetime,
timestamp_ns_to_datetime,
+ timestamp_s_to_date, timestamp_s_to_datetime, timestamp_s_to_time,
+ timestamp_us_to_datetime,
};
use chrono::DateTime;
@@ -441,4 +442,20 @@ mod tests {
assert_eq!(sec, -124);
assert_eq!(nano_sec, 999_999_999);
}
+
+ #[test]
+ fn test_time_conversions_out_of_bounds_no_wrapping() {
+ let wrapped = (1_i64 << 32) + 5;
+ assert_eq!(time64us_to_time(wrapped * 1_000_000), None);
+ assert_eq!(time64ns_to_time(wrapped * 1_000_000_000), None);
+
+ assert_eq!(time32s_to_time(86_400), None);
+ assert_eq!(time32s_to_time(-1), None);
+ assert_eq!(time32ms_to_time(86_400_000), None);
+ assert_eq!(time32ms_to_time(-1), None);
+ assert_eq!(time64us_to_time(86_400_000_000), None);
+ assert_eq!(time64us_to_time(-1), None);
+ assert_eq!(time64ns_to_time(86_400_000_000_000), None);
+ assert_eq!(time64ns_to_time(-1), None);
+ }
}
