Merge pull request #970 from gwenn/time-fmt

Fix FromSql impl for OffsetDateTime

Cargo.toml

@@ -97,7 +97,7 @@ modern-full = [
 bundled-full = ["modern-full", "bundled"]
 
 [dependencies]
-time = { version = "0.2.23", optional = true }
+time = { version = "0.3.0", features = ["formatting", "macros", "parsing"], optional = true }
 bitflags = "1.2"
 hashlink = "0.7"
 chrono = { version = "0.4", optional = true }

src/types/mod.rs

@@ -41,22 +41,24 @@ For example, to store datetimes as `i64`s counting the number of seconds since
 the Unix epoch:
 
 ```
-use rusqlite::types::{FromSql, FromSqlResult, ToSql, ToSqlOutput, ValueRef};
+use rusqlite::types::{FromSql, FromSqlError, FromSqlResult, ToSql, ToSqlOutput, ValueRef};
 use rusqlite::Result;
 
 pub struct DateTimeSql(pub time::OffsetDateTime);
 
 impl FromSql for DateTimeSql {
     fn column_result(value: ValueRef) -> FromSqlResult<Self> {
-        i64::column_result(value).map(|as_i64| {
+        i64::column_result(value).and_then(|as_i64| {
-            DateTimeSql(time::OffsetDateTime::from_unix_timestamp(as_i64))
+            time::OffsetDateTime::from_unix_timestamp(as_i64)
+            .map(|odt| DateTimeSql(odt))
+            .map_err(|err| FromSqlError::Other(Box::new(err)))
         })
     }
 }
 
 impl ToSql for DateTimeSql {
     fn to_sql(&self) -> Result<ToSqlOutput> {
-        Ok(self.0.timestamp().into())
+        Ok(self.0.unix_timestamp().into())
     }
 }
 ```

src/types/time.rs

@@ -1,16 +1,35 @@
 //! [`ToSql`] and [`FromSql`] implementation for [`time::OffsetDateTime`].
 use crate::types::{FromSql, FromSqlError, FromSqlResult, ToSql, ToSqlOutput, ValueRef};
-use crate::Result;
+use crate::{Error, Result};
+use time::format_description::well_known::Rfc3339;
+use time::format_description::FormatItem;
+use time::macros::format_description;
 use time::{OffsetDateTime, PrimitiveDateTime, UtcOffset};
 
-const CURRENT_TIMESTAMP_FMT: &str = "%Y-%m-%d %H:%M:%S";
+const PRIMITIVE_SHORT_DATE_TIME_FORMAT: &[FormatItem<'_>] =
-const SQLITE_DATETIME_FMT: &str = "%Y-%m-%d %H:%M:%S.%NZ";
+    format_description!("[year]-[month]-[day] [hour]:[minute]:[second]");
-const SQLITE_DATETIME_FMT_LEGACY: &str = "%Y-%m-%d %H:%M:%S:%N %z";
+const PRIMITIVE_DATE_TIME_FORMAT: &[FormatItem<'_>] =
+    format_description!("[year]-[month]-[day] [hour]:[minute]:[second].[subsecond]");
+const PRIMITIVE_DATE_TIME_Z_FORMAT: &[FormatItem<'_>] =
+    format_description!("[year]-[month]-[day] [hour]:[minute]:[second].[subsecond]Z");
+const OFFSET_SHORT_DATE_TIME_FORMAT: &[FormatItem<'_>] = format_description!(
+    "[year]-[month]-[day] [hour]:[minute]:[second][offset_hour sign:mandatory]:[offset_minute]"
+);
+const OFFSET_DATE_TIME_FORMAT: &[FormatItem<'_>] = format_description!(
+    "[year]-[month]-[day] [hour]:[minute]:[second].[subsecond][offset_hour sign:mandatory]:[offset_minute]"
+);
+const LEGACY_DATE_TIME_FORMAT: &[FormatItem<'_>] = format_description!(
+    "[year]-[month]-[day] [hour]:[minute]:[second]:[subsecond] [offset_hour sign:mandatory]:[offset_minute]"
+);
 
 impl ToSql for OffsetDateTime {
     #[inline]
     fn to_sql(&self) -> Result<ToSqlOutput<'_>> {
-        let time_string = self.to_offset(UtcOffset::UTC).format(SQLITE_DATETIME_FMT);
+        // FIXME keep original offset
+        let time_string = self
+            .to_offset(UtcOffset::UTC)
+            .format(&PRIMITIVE_DATE_TIME_Z_FORMAT)
+            .map_err(|err| Error::ToSqlConversionFailure(err.into()))?;
         Ok(ToSqlOutput::from(time_string))
     }
 }
@@ -18,13 +37,29 @@ impl ToSql for OffsetDateTime {
 impl FromSql for OffsetDateTime {
     fn column_result(value: ValueRef<'_>) -> FromSqlResult<Self> {
         value.as_str().and_then(|s| {
+            if s.len() > 10 && s.as_bytes()[10] == b'T' {
+                // YYYY-MM-DDTHH:MM:SS.SSS[+-]HH:MM
+                return OffsetDateTime::parse(s, &Rfc3339)
+                    .map_err(|err| FromSqlError::Other(Box::new(err)));
+            }
+            let s = s.strip_suffix('Z').unwrap_or(s);
             match s.len() {
-                19 => PrimitiveDateTime::parse(s, CURRENT_TIMESTAMP_FMT).map(|d| d.assume_utc()),
+                len if len <= 19 => {
-                _ => PrimitiveDateTime::parse(s, SQLITE_DATETIME_FMT)
+                    // TODO YYYY-MM-DDTHH:MM:SS
-                    .map(|d| d.assume_utc())
+                    PrimitiveDateTime::parse(s, &PRIMITIVE_SHORT_DATE_TIME_FORMAT)
+                        .map(|d| d.assume_utc())
+                }
+                _ if s.as_bytes()[19] == b':' => {
+                    // legacy
+                    OffsetDateTime::parse(s, &LEGACY_DATE_TIME_FORMAT)
+                }
+                _ if s.as_bytes()[19] == b'.' => OffsetDateTime::parse(s, &OFFSET_DATE_TIME_FORMAT)
                     .or_else(|err| {
-                        OffsetDateTime::parse(s, SQLITE_DATETIME_FMT_LEGACY).map_err(|_| err)
+                        PrimitiveDateTime::parse(s, &PRIMITIVE_DATE_TIME_FORMAT)
+                            .map(|d| d.assume_utc())
+                            .map_err(|_| err)
                     }),
+                _ => OffsetDateTime::parse(s, &OFFSET_SHORT_DATE_TIME_FORMAT),
             }
             .map_err(|err| FromSqlError::Other(Box::new(err)))
         })
@@ -34,23 +69,19 @@ impl FromSql for OffsetDateTime {
 #[cfg(test)]
 mod test {
     use crate::{Connection, Result};
-    use std::time::Duration;
+    use time::format_description::well_known::Rfc3339;
     use time::OffsetDateTime;
 
-    fn checked_memory_handle() -> Result<Connection> {
-        let db = Connection::open_in_memory()?;
-        db.execute_batch("CREATE TABLE foo (t TEXT, i INTEGER, f FLOAT)")?;
-        Ok(db)
-    }
-
     #[test]
     fn test_offset_date_time() -> Result<()> {
-        let db = checked_memory_handle()?;
+        let db = Connection::open_in_memory()?;
+        db.execute_batch("CREATE TABLE foo (t TEXT, i INTEGER, f FLOAT)")?;
 
         let mut ts_vec = vec![];
 
-        let make_datetime =
+        let make_datetime = |secs: i128, nanos: i128| {
-            |secs, nanos| OffsetDateTime::from_unix_timestamp(secs) + Duration::from_nanos(nanos);
+            OffsetDateTime::from_unix_timestamp_nanos(1_000_000_000 * secs + nanos).unwrap()
+        };
 
         ts_vec.push(make_datetime(10_000, 0)); //January 1, 1970 2:46:40 AM
         ts_vec.push(make_datetime(10_000, 1000)); //January 1, 1970 2:46:40 AM (and one microsecond)
@@ -71,9 +102,56 @@ mod test {
         Ok(())
     }
 
+    #[test]
+    fn test_string_values() -> Result<()> {
+        let db = Connection::open_in_memory()?;
+        for (s, t) in vec![
+            (
+                "2013-10-07 08:23:19",
+                Ok(OffsetDateTime::parse("2013-10-07T08:23:19Z", &Rfc3339).unwrap()),
+            ),
+            (
+                "2013-10-07 08:23:19Z",
+                Ok(OffsetDateTime::parse("2013-10-07T08:23:19Z", &Rfc3339).unwrap()),
+            ),
+            (
+                "2013-10-07T08:23:19Z",
+                Ok(OffsetDateTime::parse("2013-10-07T08:23:19Z", &Rfc3339).unwrap()),
+            ),
+            (
+                "2013-10-07 08:23:19.120",
+                Ok(OffsetDateTime::parse("2013-10-07T08:23:19.120Z", &Rfc3339).unwrap()),
+            ),
+            (
+                "2013-10-07 08:23:19.120Z",
+                Ok(OffsetDateTime::parse("2013-10-07T08:23:19.120Z", &Rfc3339).unwrap()),
+            ),
+            (
+                "2013-10-07T08:23:19.120Z",
+                Ok(OffsetDateTime::parse("2013-10-07T08:23:19.120Z", &Rfc3339).unwrap()),
+            ),
+            (
+                "2013-10-07 04:23:19-04:00",
+                Ok(OffsetDateTime::parse("2013-10-07T04:23:19-04:00", &Rfc3339).unwrap()),
+            ),
+            (
+                "2013-10-07 04:23:19.120-04:00",
+                Ok(OffsetDateTime::parse("2013-10-07T04:23:19.120-04:00", &Rfc3339).unwrap()),
+            ),
+            (
+                "2013-10-07T04:23:19.120-04:00",
+                Ok(OffsetDateTime::parse("2013-10-07T04:23:19.120-04:00", &Rfc3339).unwrap()),
+            ),
+        ] {
+            let result: Result<OffsetDateTime> = db.query_row("SELECT ?", [s], |r| r.get(0));
+            assert_eq!(result, t);
+        }
+        Ok(())
+    }
+
     #[test]
     fn test_sqlite_functions() -> Result<()> {
-        let db = checked_memory_handle()?;
+        let db = Connection::open_in_memory()?;
         let result: Result<OffsetDateTime> =
             db.query_row("SELECT CURRENT_TIMESTAMP", [], |r| r.get(0));
         assert!(result.is_ok());
@@ -82,7 +160,7 @@ mod test {
 
     #[test]
     fn test_param() -> Result<()> {
-        let db = checked_memory_handle()?;
+        let db = Connection::open_in_memory()?;
         let result: Result<bool> = db.query_row("SELECT 1 WHERE ? BETWEEN datetime('now', '-1 minute') AND datetime('now', '+1 minute')", [OffsetDateTime::now_utc()], |r| r.get(0));
         assert!(result.is_ok());
         Ok(())