This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11899-dc59495122884e4aced624e6744f268131472094 in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
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); + } }
