Add Jiff support #1164
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| use bytes::BytesMut; | ||
| use jiff_01::{ | ||
| civil::{Date, DateTime, Time}, | ||
| tz::TimeZone, | ||
| Span, Timestamp as JiffTimestamp, Zoned, | ||
| }; | ||
| use postgres_protocol::types; | ||
| use std::error::Error; | ||
|
|
||
| use crate::{FromSql, IsNull, ToSql, Type}; | ||
|
|
||
| const fn base() -> DateTime { | ||
| DateTime::constant(2000, 1, 1, 0, 0, 0, 0) | ||
| } | ||
|
|
||
| impl<'a> FromSql<'a> for DateTime { | ||
| fn from_sql(_: &Type, raw: &[u8]) -> Result<DateTime, Box<dyn Error + Sync + Send>> { | ||
| let t = types::timestamp_from_sql(raw)?; | ||
| Ok(base().checked_add(Span::new().microseconds(t))?) | ||
| } | ||
|
|
||
| accepts!(TIMESTAMP); | ||
| } | ||
|
|
||
| impl ToSql for DateTime { | ||
| fn to_sql(&self, _: &Type, w: &mut BytesMut) -> Result<IsNull, Box<dyn Error + Sync + Send>> { | ||
| types::timestamp_to_sql(self.since(base())?.get_microseconds(), w); | ||
| Ok(IsNull::No) | ||
| } | ||
|
|
||
| accepts!(TIMESTAMP); | ||
| to_sql_checked!(); | ||
| } | ||
|
|
||
| impl<'a> FromSql<'a> for JiffTimestamp { | ||
| fn from_sql(type_: &Type, raw: &[u8]) -> Result<JiffTimestamp, Box<dyn Error + Sync + Send>> { | ||
| Ok(DateTime::from_sql(type_, raw)? | ||
| .to_zoned(TimeZone::UTC)? | ||
| .timestamp()) | ||
| } | ||
|
|
||
| accepts!(TIMESTAMPTZ); | ||
| } | ||
|
|
||
| impl ToSql for JiffTimestamp { | ||
| fn to_sql(&self, _: &Type, w: &mut BytesMut) -> Result<IsNull, Box<dyn Error + Sync + Send>> { | ||
| types::timestamp_to_sql( | ||
| self.since(base().to_zoned(TimeZone::UTC)?)? | ||
| .get_microseconds(), | ||
| w, | ||
| ); | ||
| Ok(IsNull::No) | ||
| } | ||
|
|
||
| accepts!(TIMESTAMPTZ); | ||
| to_sql_checked!(); | ||
| } | ||
|
|
||
| impl<'a> FromSql<'a> for Zoned { | ||
| fn from_sql(type_: &Type, raw: &[u8]) -> Result<Zoned, Box<dyn Error + Sync + Send>> { | ||
| Ok(JiffTimestamp::from_sql(type_, raw)?.to_zoned(TimeZone::UTC)) | ||
|
There was a problem hiding this comment. Is it correct to assume UTC here? There is no time zone information in PostgreSQL? I wonder if it makes sense to leave out the impl for There was a problem hiding this comment. UTC is correct. From the Postgres docs:
Consumers will usually reach for There was a problem hiding this comment. I'm skeptical that a I understand you're starting from a position of "this is how the Chrono integration works," but Chrono's The only trait implementation I feel 100% confident about here is for There was a problem hiding this comment. I have always felt weird about how Postgres handles time zones and how the I am not sure I agree with your civil type concerns though. I feel like the Postgres types match up quite nicely for these inexact expressions of time (Postgres also has a There was a problem hiding this comment. RE
I'm going based on the code in this PR. What I see is that a PostgreSQL timestamp (which is exact time) is being convert to and from a Jiff AIUI, PostgreSQL does have Now that I look closer at this patch, I see that a Basically, any time you move between exact and inexact time, you should have some kind of time zone assertion involved. And usually, "just use UTC" is not the right thing to do.
Noooooooooo. See: https://wiki.postgresql.org/wiki/Don%27t_Do_This#Don.27t_use_timetz There was a problem hiding this comment.
There was a problem hiding this comment.
Whoa. OK. Let me backup and read their docs more carefully... Because if a
Wild. I had assumed that
I think where I got confused is that in this patch, the And yes, it looks like the above mapping lines up with what you have in this PR. OK, sorry for the noise! There was a problem hiding this comment.
It's certainly confusing.
For the record,
Comments were much appreciated :) At least we're only dealing with Terran time. |
||
| } | ||
|
|
||
| accepts!(TIMESTAMPTZ); | ||
| } | ||
|
|
||
| impl ToSql for Zoned { | ||
| fn to_sql( | ||
| &self, | ||
| type_: &Type, | ||
| w: &mut BytesMut, | ||
| ) -> Result<IsNull, Box<dyn Error + Sync + Send>> { | ||
| self.timestamp().to_sql(type_, w) | ||
| } | ||
|
|
||
| accepts!(TIMESTAMPTZ); | ||
| to_sql_checked!(); | ||
| } | ||
|
|
||
| impl<'a> FromSql<'a> for Date { | ||
| fn from_sql(_: &Type, raw: &[u8]) -> Result<Date, Box<dyn Error + Sync + Send>> { | ||
| let jd = types::date_from_sql(raw)?; | ||
| Ok(base().date().checked_add(Span::new().days(jd))?) | ||
| } | ||
|
|
||
| accepts!(DATE); | ||
| } | ||
|
|
||
| impl ToSql for Date { | ||
| fn to_sql(&self, _: &Type, w: &mut BytesMut) -> Result<IsNull, Box<dyn Error + Sync + Send>> { | ||
| let jd = self.since(base().date())?.get_days(); | ||
| types::date_to_sql(jd, w); | ||
| Ok(IsNull::No) | ||
| } | ||
|
|
||
| accepts!(DATE); | ||
| to_sql_checked!(); | ||
| } | ||
|
|
||
| impl<'a> FromSql<'a> for Time { | ||
| fn from_sql(_: &Type, raw: &[u8]) -> Result<Time, Box<dyn Error + Sync + Send>> { | ||
| let usec = types::time_from_sql(raw)?; | ||
| Ok(Time::midnight() + Span::new().microseconds(usec)) | ||
| } | ||
|
|
||
| accepts!(TIME); | ||
| } | ||
|
|
||
| impl ToSql for Time { | ||
| fn to_sql(&self, _: &Type, w: &mut BytesMut) -> Result<IsNull, Box<dyn Error + Sync + Send>> { | ||
| let delta = self.since(Time::midnight())?; | ||
| types::time_to_sql(delta.get_microseconds(), w); | ||
| Ok(IsNull::No) | ||
| } | ||
|
|
||
| accepts!(TIME); | ||
| to_sql_checked!(); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not intricately familiar with how PostgreSQL represents time, but it seems like you can avoid going through a
DateTimehere and just compute aJiffTimestampdirectly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in f00d208