temporal conversion functions should work on negative input properly (#…

…2326)

* For negative timestamp

* Fix clippy

* Add test

arrow/src/temporal_conversions.rs

@@ -42,11 +42,13 @@ pub fn date32_to_datetime(v: i32) -> NaiveDateTime {
 /// converts a `i64` representing a `date64` to [`NaiveDateTime`]
 #[inline]
 pub fn date64_to_datetime(v: i64) -> NaiveDateTime {
+    let (sec, milli_sec) = split_second(v, MILLISECONDS);
+
     NaiveDateTime::from_timestamp(
         // extract seconds from milliseconds
-        v / MILLISECONDS,
+        sec,
         // discard extracted seconds and convert milliseconds to nanoseconds
-        (v % MILLISECONDS * MICROSECONDS) as u32,
+        milli_sec * MICROSECONDS as u32,
     )
 }
 
@@ -101,36 +103,59 @@ pub fn timestamp_s_to_datetime(v: i64) -> NaiveDateTime {
 /// converts a `i64` representing a `timestamp(ms)` to [`NaiveDateTime`]
 #[inline]
 pub fn timestamp_ms_to_datetime(v: i64) -> NaiveDateTime {
+    let (sec, milli_sec) = split_second(v, MILLISECONDS);
+
     NaiveDateTime::from_timestamp(
         // extract seconds from milliseconds
-        v / MILLISECONDS,
+        sec,
         // discard extracted seconds and convert milliseconds to nanoseconds
-        (v % MILLISECONDS * MICROSECONDS) as u32,
+        milli_sec * MICROSECONDS as u32,
     )
 }
 
 /// converts a `i64` representing a `timestamp(us)` to [`NaiveDateTime`]
 #[inline]
 pub fn timestamp_us_to_datetime(v: i64) -> NaiveDateTime {
+    let (sec, micro_sec) = split_second(v, MICROSECONDS);
+
     NaiveDateTime::from_timestamp(
         // extract seconds from microseconds
-        v / MICROSECONDS,
+        sec,
         // discard extracted seconds and convert microseconds to nanoseconds
-        (v % MICROSECONDS * MILLISECONDS) as u32,
+        micro_sec * MILLISECONDS as u32,
     )
 }
 
 /// converts a `i64` representing a `timestamp(ns)` to [`NaiveDateTime`]
 #[inline]
 pub fn timestamp_ns_to_datetime(v: i64) -> NaiveDateTime {
+    let (sec, nano_sec) = split_second(v, NANOSECONDS);
+
     NaiveDateTime::from_timestamp(
         // extract seconds from nanoseconds
-        v / NANOSECONDS,
-        // discard extracted seconds
-        if v > 0 { (v % NANOSECONDS) as u32 } else { 0 },
+        sec, // discard extracted seconds
+        nano_sec,
     )
 }
 
+///
+#[inline]
+pub(crate) fn split_second(v: i64, base: i64) -> (i64, u32) {
+    if v < 0 {
+        let v = -v;
+        let mut seconds = v / base;
+        let mut part = v % base;
+
+        if part > 0 {
+            seconds += 1;
+            part = base - part;
+        }
+        (-seconds, part as u32)
+    } else {
+        (v / base, (v % base) as u32)
+    }
+}
+
 /// converts a `i64` representing a `duration(s)` to [`Duration`]
 #[inline]
 pub fn duration_s_to_duration(v: i64) -> Duration {
@@ -154,3 +179,83 @@ pub fn duration_us_to_duration(v: i64) -> Duration {
 pub fn duration_ns_to_duration(v: i64) -> Duration {
     Duration::nanoseconds(v)
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::temporal_conversions::{
+        date64_to_datetime, split_second, timestamp_ms_to_datetime,
+        timestamp_ns_to_datetime, timestamp_us_to_datetime, NANOSECONDS,
+    };
+    use chrono::NaiveDateTime;
+
+    #[test]
+    fn negative_input_timestamp_ns_to_datetime() {
+        assert_eq!(
+            timestamp_ns_to_datetime(-1),
+            NaiveDateTime::from_timestamp(-1, 999_999_999)
+        );
+
+        assert_eq!(
+            timestamp_ns_to_datetime(-1_000_000_001),
+            NaiveDateTime::from_timestamp(-2, 999_999_999)
+        );
+    }
+
+    #[test]
+    fn negative_input_timestamp_us_to_datetime() {
+        assert_eq!(
+            timestamp_us_to_datetime(-1),
+            NaiveDateTime::from_timestamp(-1, 999_999_000)
+        );
+
+        assert_eq!(
+            timestamp_us_to_datetime(-1_000_001),
+            NaiveDateTime::from_timestamp(-2, 999_999_000)
+        );
+    }
+
+    #[test]
+    fn negative_input_timestamp_ms_to_datetime() {
+        assert_eq!(
+            timestamp_ms_to_datetime(-1),
+            NaiveDateTime::from_timestamp(-1, 999_000_000)
+        );
+
+        assert_eq!(
+            timestamp_ms_to_datetime(-1_001),
+            NaiveDateTime::from_timestamp(-2, 999_000_000)
+        );
+    }
+
+    #[test]
+    fn negative_input_date64_to_datetime() {
+        assert_eq!(
+            date64_to_datetime(-1),
+            NaiveDateTime::from_timestamp(-1, 999_000_000)
+        );
+
+        assert_eq!(
+            date64_to_datetime(-1_001),
+            NaiveDateTime::from_timestamp(-2, 999_000_000)
+        );
+    }
+
+    #[test]
+    fn test_split_seconds() {
+        let (sec, nano_sec) = split_second(100, NANOSECONDS);
+        assert_eq!(sec, 0);
+        assert_eq!(nano_sec, 100);
+
+        let (sec, nano_sec) = split_second(123_000_000_456, NANOSECONDS);
+        assert_eq!(sec, 123);
+        assert_eq!(nano_sec, 456);
+
+        let (sec, nano_sec) = split_second(-1, NANOSECONDS);
+        assert_eq!(sec, -1);
+        assert_eq!(nano_sec, 999_999_999);
+
+        let (sec, nano_sec) = split_second(-123_000_000_001, NANOSECONDS);
+        assert_eq!(sec, -124);
+        assert_eq!(nano_sec, 999_999_999);
+    }
+}