Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[draft] Implement To/FromSql for chrono::DateTime with SQLite #2351

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[draft] Implement To/FromSql for chrono::DateTime with SQLite #2351

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
235 changes: 233 additions & 2 deletions diesel/src/sqlite/types/date_and_time/chrono.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
extern crate chrono;

use self::chrono::{NaiveDate, NaiveDateTime, NaiveTime};
use self::chrono::{DateTime, FixedOffset, NaiveDate, NaiveDateTime, NaiveTime};
use std::io::Write;

use crate::backend;
Expand Down Expand Up @@ -103,12 +103,31 @@ impl ToSql<Timestamp, Sqlite> for NaiveDateTime {
}
}

// chrono::DateTime<FixedOffset> impls

impl FromSql<Text, Sqlite> for DateTime<FixedOffset> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have made that probably clearer in #2331, but we definitively don't want to have literally impl FromSql<Text, Sqlite> DateTime<FixedOffset> here, because that would:

  • Remove any type safety
  • Interact poorly with the strongly typed query builder

So to clarify that a bit more, I think a better approach to handle this is to use a distinct SQL type for this. I'm not 100% sold to reusing postgres Timestamptz type here as I'm not sure if the semantics are similar, but that could be a solution. Alternatively we could introduce a new sql type. So the strategy should be basically similar to that one above used to implement support for NaiveDateTime.
Other things I've noticed while thinking about this:

  • We are using here some guessing for the actual timestamp format. It's probably a good idea to do that here as well with all formats that support timezone information. So we can be compatible with all sqlite supported formats and not only rfc3339 style timestamps.
  • We probably want to support DateTime<TZ: Timezone> instead of only DateTime<FixedOffset>, similar to the postgres variant.

fn from_sql(value: Option<backend::RawValue<Sqlite>>) -> deserialize::Result<Self> {
let text_ptr = <*const str as FromSql<Text, Sqlite>>::from_sql(value)?;
let text = unsafe { &*text_ptr };

Self::parse_from_rfc3339(&text).map_err(Into::into)
}
}

impl ToSql<Text, Sqlite> for DateTime<FixedOffset> {
fn to_sql<W: Write>(&self, out: &mut Output<W, Sqlite>) -> serialize::Result {
let s = self.to_rfc3339();
ToSql::<Text, Sqlite>::to_sql(&s, out)
}
}


#[cfg(test)]
mod tests {
extern crate chrono;
extern crate dotenv;

use self::chrono::{Duration, NaiveDate, NaiveDateTime, NaiveTime, Timelike, Utc};
use self::chrono::{DateTime, Duration, FixedOffset, NaiveDate, NaiveDateTime, NaiveTime, Timelike, TimeZone, Utc};
use self::dotenv::dotenv;

use crate::dsl::{now, sql};
Expand Down Expand Up @@ -375,4 +394,216 @@ mod tests {
let query = select(datetime("9999-01-08 00:00:00.000000").eq(distant_future));
assert!(query.get_result::<bool>(&connection).unwrap());
}

// chrono::DateTime decode/encode tests

#[test]
/// Test that a rfc3339-formatted chrono::DateTime decodes correctly.
fn rfc3339_datetime_decodes_correctly() {
let connection = connection();

// from datetime_decodes_correctly for NaiveDateTime, but with Utc timezone

let january_first_2000: DateTime<FixedOffset> = Utc
.ymd(2000, 1, 1)
.and_hms(1, 1, 1).into();
let query = select(sql::<Text>("'2000-01-01T01:01:01.000000Z'"));
assert_eq!(
Ok(january_first_2000),
query.get_result::<DateTime<FixedOffset>>(&connection)
);

let distant_past: DateTime<FixedOffset> = Utc
.ymd(0, 4, 11)
.and_hms(2, 2, 2).into();
let query = select(sql::<Text>("'0000-04-11t02:02:02.000000Z'"));
assert_eq!(
Ok(distant_past),
query.get_result::<DateTime<FixedOffset>>(&connection)
);

let january_first_2018: DateTime<FixedOffset> = Utc
.ymd(2018, 1, 1)
.and_hms(1, 1, 1).into();
let query = select(sql::<Text>("'2018-01-01T01:01:01.00Z'"));
assert_eq!(
Ok(january_first_2018),
query.get_result::<DateTime<FixedOffset>>(&connection)
);

let distant_future: DateTime<FixedOffset> = Utc
.ymd(9999, 1, 8)
.and_hms_nano(23, 59, 59, 100_000).into();
let query = select(sql::<Text>("'9999-01-08t23:59:59.000100z'"));
assert_eq!(
Ok(distant_future),
query.get_result::<DateTime<FixedOffset>>(&connection)
);

// same test cases as above with different timezones
let hour: i32 = 3600;

let january_first_2000: DateTime<FixedOffset> = FixedOffset::east(hour * 4)
.ymd(2000, 1, 1)
.and_hms(1, 1, 1).into();
let query = select(sql::<Text>("'2000-01-01T01:01:01.0+04:00'"));
assert_eq!(
Ok(january_first_2000),
query.get_result::<DateTime<FixedOffset>>(&connection)
);

let distant_past: DateTime<FixedOffset> = FixedOffset::east(hour * -3)
.ymd(0, 4, 11)
.and_hms(2, 2, 2).into();
let query = select(sql::<Text>("'0000-04-11t02:02:02.000000-03:00'"));
assert_eq!(
Ok(distant_past),
query.get_result::<DateTime<FixedOffset>>(&connection)
);

let january_first_2018: DateTime<FixedOffset> = FixedOffset::east(hour * 12)
.ymd(2018, 1, 1)
.and_hms(1, 1, 1).into();
let query = select(sql::<Text>("'2018-01-01T01:01:01.00+12:00'"));
assert_eq!(
Ok(january_first_2018),
query.get_result::<DateTime<FixedOffset>>(&connection)
);

let distant_future: DateTime<FixedOffset> = FixedOffset::east(hour * -7 + (hour/2))
.ymd(9999, 1, 8)
.and_hms_nano(23, 59, 59, 100_000).into();
let query = select(sql::<Text>("'9999-01-08t23:59:59.000100-06:30'"));
assert_eq!(
Ok(distant_future),
query.get_result::<DateTime<FixedOffset>>(&connection)
);
}

#[test]
/// Test that an incorrectly-formatted rfc3339 timestamp decodes with an error.
fn invalid_rfc3339_datetime_decodes_error() {
let connection = connection();

let query = select(sql::<Text>("''"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_err(), "Empty timestamp was parsed into valid datetime: {}", res.unwrap().to_string());

let query = select(sql::<Text>("'x'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_err(), "Invalid timestamp was parsed into valid datetime: {}", res.unwrap().to_string());

let query = select(sql::<Text>("'x'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_err(), "Invalid timestamp was parsed into valid datetime: {}", res.unwrap().to_string());

let query = select(sql::<Text>("'9999-01-08 23:59:59.000100'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_err(), "Timestamp with no timezone was parsed into valid datetime: {}", res.unwrap().to_string());

let query = select(sql::<Text>("'2000-01-01T01:01:01.000000+25:00'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_err(), "Timestamp with invalid timezone was parsed into valid datetime: {}", res.unwrap().to_string());

let query = select(sql::<Text>("'Wed, 18 Feb 2015 23:16:09 GMT'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_err(), "RFC 2822 timestamp (lol) was parsed into valid datetime: {}", res.unwrap().to_string());
}

#[test]
/// Test rfc3339 datetimes encode correctly. Fewer test cases here because we're literally just
/// storing it as a string.
fn rfc3339_datetimes_encode_correctly() {
let connection = connection();
let january_first_2000: DateTime<FixedOffset> = Utc
.ymd(2000, 1, 1)
.and_hms(0, 0, 0).into();

// select the rfc3339 string as text, extract the result as a chrono::DateTime, check no
// error, and then check that it deserialized to the correct datetime. All of the tests
// below follow this pattern.
let query = select(sql::<Text>("'2000-01-01T00:00:00.000000Z'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_ok(), "Error selecting datetime: {}", res.unwrap_err());

let dt = res.unwrap();
assert_eq!(dt, january_first_2000);


// same as above, checking that regardless of timezone storage (Z or +/-00:00), the resulting DateTime is
// the same.
let query = select(sql::<Text>("'2000-01-01T00:00:00.000000+00:00'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_ok(), "Error selecting datetime: {}", res.unwrap_err());

let dt = res.unwrap();
assert_eq!(dt, january_first_2000);


let query = select(sql::<Text>("'2000-01-01T00:00:00.000000-00:00'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_ok(), "Error selecting datetime: {}", res.unwrap_err());

let dt = res.unwrap();
assert_eq!(dt, january_first_2000);


let distant_past: DateTime<FixedOffset> = Utc
.ymd(0, 4, 11)
.and_hms(20, 00, 20).into();
let query = select(sql::<Text>("'0000-04-11t20:00:20.000000Z'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_ok(), "Error selecting datetime: {}", res.unwrap_err());

let dt = res.unwrap();
assert_eq!(dt, distant_past);


let january_first_2018 = FixedOffset::east(30*60)
.ymd(2018, 1, 1)
.and_hms_nano(12, 00, 00, 500_000);
let query = select(sql::<Text>("'2018-01-01t12:00:00.000500+00:30'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_ok(), "Error selecting datetime: {}", res.unwrap_err());

let dt = res.unwrap();
assert_eq!(dt, january_first_2018);


let distant_future = FixedOffset::west(3600)
.ymd(9999, 1, 8)
.and_hms(0, 0, 0);
let query = select(sql::<Text>("'9999-01-08t00:00:00.000000-01:00'"));
let res = query.get_result::<DateTime<FixedOffset>>(&connection);
assert!(res.is_ok(), "Error selecting datetime: {}", res.unwrap_err());

let dt = res.unwrap();
assert_eq!(dt, distant_future);
}

// #[test]
// /// Test that the sql select...where ordering functionality interops properly with rfc3339 datetimes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those two tests should just "work" if we use Timestamptz as sql type. Making them work with Text as sql type is basically impossible.

// fn rfc3339_datetimes_relative_to_now_encode_correctly() {
// let connection = connection();
// let time: DateTime<FixedOffset> = (Utc::now() + Duration::seconds(60)).into();
// let query = select(now.lt(time));
// assert_eq!(Ok(true), query.get_result(&connection));

// let time: DateTime<FixedOffset> = (Utc::now() - Duration::seconds(60)).into();
// let query = select(now.gt(time));
// assert_eq!(Ok(true), query.get_result(&connection));
// }

// #[test]
// fn rfc3339_datetimes_order_by_properly() {
// let connection = connection();
// let now = Utc::now();
// let now_est = Utc::now().with_timezone(FixedOffset::west(3600 * 5));
// let past: DateTime<FixedOffset> = (now - Duration::seconds(60)).into();
// let future: DateTime<FixedOffset> = (now + Duration::seconds(60)).into();

// // TODO: how do I select multiple row/value literals to test order_by?
// // select "literal1" UNION ALL select "literal2" [...]?

// }
}