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.

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

feat: bootstrap jiff-sqlx development #141

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ members = [
"jiff-cli",
"jiff-tzdb",
"jiff-tzdb-platform",
"jiff-sqlx",
"examples/*",
]

Expand Down
21 changes: 21 additions & 0 deletions jiff-sqlx/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[package]
name = "jiff-sqlx"
version = "0.1.0"
license = "Unlicense OR MIT"
homepage = "https://github.com/BurntSushi/jiff/tree/master/jiff-sqlx"
repository = "https://github.com/BurntSushi/jiff"
documentation = "https://docs.rs/jiff-sqlx"
description = "Integration to use jiff structs for datetime types in sqlx."
categories = ["date-and-time"]
keywords = ["date", "time", "temporal", "zone", "iana"]
workspace = ".."
edition = "2021"
rust-version = "1.70"

[features]
default = []
postgres = ["sqlx/postgres"]

[dependencies]
jiff = { path = ".." }
sqlx = { version = "0.8", default-features = false }
5 changes: 5 additions & 0 deletions jiff-sqlx/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[cfg(feature = "postgres")]
mod postgres;

mod wrap_types;
pub use wrap_types::*;
63 changes: 63 additions & 0 deletions jiff-sqlx/src/postgres/date.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
use crate::{Date, ToDate};
use sqlx::encode::IsNull;
use sqlx::error::BoxDynError;
use sqlx::postgres::types::Oid;
use sqlx::postgres::{
PgArgumentBuffer, PgHasArrayType, PgTypeInfo, PgValueFormat, PgValueRef,
};
use sqlx::{Decode, Encode, Postgres, Type};

impl Type<Postgres> for Date {
fn type_info() -> PgTypeInfo {
// 1082 => PgType::Date
PgTypeInfo::with_oid(Oid(1082))
}
}

impl PgHasArrayType for Date {
fn array_type_info() -> PgTypeInfo {
// 1182 => PgType::DateArray
PgTypeInfo::with_oid(Oid(1182))
}
}

impl Encode<'_, Postgres> for Date {
fn encode_by_ref(
&self,
buf: &mut PgArgumentBuffer,
) -> Result<IsNull, BoxDynError> {
let date = self.to_jiff();

// DATE is encoded as the days since epoch
let days = date.since(postgres_epoch_date())?.get_days();
Encode::<Postgres>::encode(days, buf)
}

fn size_hint(&self) -> usize {
size_of::<i32>()
}
}

impl<'r> Decode<'r, Postgres> for Date {
fn decode(value: PgValueRef<'r>) -> Result<Self, BoxDynError> {
Ok(match value.format() {
PgValueFormat::Binary => {
// DATE is encoded as the days since epoch
let days: i32 = Decode::<Postgres>::decode(value)?;
let date = jiff::Span::new()
.try_days(days)
.and_then(|s| postgres_epoch_date().checked_add(s))?;
date.to_sqlx()
}
PgValueFormat::Text => {
let s = value.as_str()?;
let date = jiff::civil::Date::strptime("%Y-%m-%d", s)?;
date.to_sqlx()
}
})
}
}

const fn postgres_epoch_date() -> jiff::civil::Date {
jiff::civil::Date::constant(2000, 1, 1)
}
68 changes: 68 additions & 0 deletions jiff-sqlx/src/postgres/datetime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use crate::{DateTime, ToDateTime};
use jiff::SignedDuration;
use sqlx::encode::IsNull;
use sqlx::error::BoxDynError;
use sqlx::postgres::types::Oid;
use sqlx::postgres::{
PgArgumentBuffer, PgHasArrayType, PgTypeInfo, PgValueFormat, PgValueRef,
};
use sqlx::{Decode, Encode, Postgres, Type};
use std::str::FromStr;

impl Type<Postgres> for DateTime {
fn type_info() -> PgTypeInfo {
// 1114 => PgType::Timestamp
PgTypeInfo::with_oid(Oid(1114))
}
}

impl PgHasArrayType for DateTime {
fn array_type_info() -> PgTypeInfo {
// 1115 => PgType::TimestampArray
PgTypeInfo::with_oid(Oid(1115))
}
}

impl Encode<'_, Postgres> for DateTime {
fn encode_by_ref(
&self,
buf: &mut PgArgumentBuffer,
) -> Result<IsNull, BoxDynError> {
let datetime = self.to_jiff();

// TIMESTAMP is encoded as the microseconds since the epoch
let micros =
datetime.duration_since(postgres_epoch_datetime()).as_micros();
let micros = i64::try_from(micros).map_err(|_| {
format!("DateTime {datetime} out of range for Postgres: {micros}")
})?;
Encode::<Postgres>::encode(micros, buf)
}

fn size_hint(&self) -> usize {
size_of::<i64>()
}
}

impl<'r> Decode<'r, Postgres> for DateTime {
fn decode(value: PgValueRef<'r>) -> Result<Self, BoxDynError> {
Ok(match value.format() {
PgValueFormat::Binary => {
// TIMESTAMP is encoded as the microseconds since the epoch
let us = Decode::<Postgres>::decode(value)?;
let datetime = postgres_epoch_datetime()
.checked_add(SignedDuration::from_micros(us))?;
datetime.to_sqlx()
}
PgValueFormat::Text => {
let s = value.as_str()?;
let datetime = jiff::civil::DateTime::from_str(s)?;
datetime.to_sqlx()
}
})
}
}

const fn postgres_epoch_datetime() -> jiff::civil::DateTime {
jiff::civil::DateTime::constant(2000, 1, 1, 0, 0, 0, 0)
}
133 changes: 133 additions & 0 deletions jiff-sqlx/src/postgres/interval.rs
Copy link
Author

Choose a reason for hiding this comment

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

@maxcountryman I'd re-add the impl for decoding SignedDuration.

I saw your reaction on the comment but unsure what the concrete issue you found. Maybe you can reply here.

For support of Span and convert between Span and PgInterval for sqlx codec, I'd leave it to the next PR if we get progress here, or there is a real usage requirement.

Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
use crate::SignedDuration;
use sqlx::encode::IsNull;
use sqlx::error::BoxDynError;
use sqlx::postgres::types::{Oid, PgInterval};
use sqlx::postgres::{PgArgumentBuffer, PgHasArrayType, PgTypeInfo};
use sqlx::{Database, Decode, Encode, Postgres, Type};

impl Type<Postgres> for SignedDuration {
fn type_info() -> PgTypeInfo {
// 1186 => PgType::Interval
PgTypeInfo::with_oid(Oid(1186))
}
}

impl PgHasArrayType for SignedDuration {
fn array_type_info() -> PgTypeInfo {
// 1187 => PgType::IntervalArray
PgTypeInfo::with_oid(Oid(1187))
}
}

impl TryFrom<SignedDuration> for PgInterval {
type Error = BoxDynError;

/// Convert a `SignedDuration` to a `PgInterval`.
///
/// This returns an error if there is a loss of precision using nanoseconds or if there is a
/// microseconds overflow.
fn try_from(value: SignedDuration) -> Result<Self, BoxDynError> {
let value = value.to_jiff();

if value.subsec_nanos() % 1000 != 0 {
return Err(
"PostgreSQL `INTERVAL` does not support nanoseconds precision"
.into(),
);
}

let micros = value.as_micros();
if micros >= i64::MIN as i128 && micros <= i64::MAX as i128 {
Ok(Self { months: 0, days: 0, microseconds: micros as i64 })
} else {
Err("Overflow has occurred for PostgreSQL `INTERVAL`".into())
}
}
}

impl Encode<'_, Postgres> for SignedDuration {
fn encode_by_ref(
&self,
buf: &mut PgArgumentBuffer,
) -> Result<IsNull, BoxDynError> {
let pg_interval = PgInterval::try_from(*self)?;
pg_interval.encode_by_ref(buf)
}

fn size_hint(&self) -> usize {
2 * size_of::<i64>()
}
}

impl<'r> Decode<'r, Postgres> for SignedDuration {
fn decode(
value: <Postgres as Database>::ValueRef<'r>,
) -> Result<Self, BoxDynError> {
let pg_interval = PgInterval::decode(value)?;

if pg_interval.months != 0 {
return Err(
"Cannot convert months in `INTERVAL` to SignedDuration".into(),
);
}

if pg_interval.days != 0 {
return Err(
"Cannot convert days in `INTERVAL` to SignedDuration".into()
);
}

let micros = pg_interval.microseconds;
Copy link
Owner

Choose a reason for hiding this comment

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

@maxcountryman Ah so I think this is where there is a correctness issue? The non-microseconds units are being silently ignored here.

Choose a reason for hiding this comment

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

This is almost certainly incorrect, yes.

However, I'm also pointing out that Postgres can be configured to output ISO 8601. Your docs say SignedDuration is not capable of parsing ISO 8601 with non-zero days. Moreover, my impression is that Spans may be preferred for ISO 8601. Regardless, Postgres will use non-zero days when configured this way so that must be handled somehow. In my own programs I just use Span.

I said as much earlier in the thread but the original author did not respond after I pointed this out.

I don't think an implementation that doesn't address this somehow should be published personally.

Choose a reason for hiding this comment

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

Just to confirm there is a correctness issue here, this implementation omits two fields, days and months.

Copy link
Owner

Choose a reason for hiding this comment

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

Okay thanks, I understand now. I agree.

I'll take a closer look later.

Copy link
Author

@tisonkun tisonkun Jan 5, 2025

Choose a reason for hiding this comment

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

@maxcountryman Thanks for your feedback! Your analysis is correct.

The decode part is added three days ago at 2f17955 while I'd like to use SignedDuration to hold PG's interval. The background in my use case is that all the interval is written by my code and thus it's guaranteed only microseconds unit are filled. But for general purpose usage, this may not be true.

There is a related discussion at #174 (reply in thread). In short, I'd first remove the decode impl for SignedDuration now because you can hardly convert days & months to seconds without a relative datetime by jiff's design. But perhaps we can support codec over Span.

However, the encode part of SignedDuration should be fine.

Copy link
Author

Choose a reason for hiding this comment

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

Or instead, we may check months and days are 0 when decode to SignedDuration and return an error if so.

Ok(SignedDuration(jiff::SignedDuration::from_micros(micros)))
}
}

#[cfg(test)]
mod tests {
use crate::ToSignedDuration;
use sqlx::postgres::types::PgInterval;

#[test]
fn test_pginterval_jiff() {
// Case for positive duration
let interval = PgInterval { days: 0, months: 0, microseconds: 27_000 };
assert_eq!(
&PgInterval::try_from(
jiff::SignedDuration::from_micros(27_000).to_sqlx()
)
.unwrap(),
&interval
);

// Case for negative duration
let interval =
PgInterval { days: 0, months: 0, microseconds: -27_000 };
assert_eq!(
&PgInterval::try_from(
jiff::SignedDuration::from_micros(-27_000).to_sqlx()
)
.unwrap(),
&interval
);

// Case when precision loss occurs
assert!(PgInterval::try_from(
jiff::SignedDuration::from_nanos(27_000_001).to_sqlx()
)
.is_err());
assert!(PgInterval::try_from(
jiff::SignedDuration::from_nanos(-27_000_001).to_sqlx()
)
.is_err());

// Case when microseconds overflow occurs
assert!(PgInterval::try_from(
jiff::SignedDuration::from_secs(10_000_000_000_000).to_sqlx()
)
.is_err());
assert!(PgInterval::try_from(
jiff::SignedDuration::from_secs(-10_000_000_000_000).to_sqlx()
)
.is_err());
}
}
5 changes: 5 additions & 0 deletions jiff-sqlx/src/postgres/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
mod date;
mod datetime;
mod interval;
mod time;
mod timestamp;
63 changes: 63 additions & 0 deletions jiff-sqlx/src/postgres/time.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
use crate::{Time, ToTime};
use jiff::SignedDuration;
use sqlx::encode::IsNull;
use sqlx::error::BoxDynError;
use sqlx::postgres::types::Oid;
use sqlx::postgres::{
PgArgumentBuffer, PgHasArrayType, PgTypeInfo, PgValueFormat, PgValueRef,
};
use sqlx::{Decode, Encode, Postgres, Type};

impl Type<Postgres> for Time {
fn type_info() -> PgTypeInfo {
// 1083 => PgType::Time
PgTypeInfo::with_oid(Oid(1083))
}
}

impl PgHasArrayType for Time {
fn array_type_info() -> PgTypeInfo {
// 1183 => PgType::TimeArray
PgTypeInfo::with_oid(Oid(1183))
}
}

impl Encode<'_, Postgres> for Time {
fn encode_by_ref(
&self,
buf: &mut PgArgumentBuffer,
) -> Result<IsNull, BoxDynError> {
let time = self.to_jiff();

// TIME is encoded as the microseconds since midnight
let micros =
time.duration_since(jiff::civil::Time::midnight()).as_micros();
let micros = i64::try_from(micros).map_err(|_| {
format!("Time {time} out of range for Postgres: {micros}")
})?;
Encode::<Postgres>::encode(micros, buf)
}

fn size_hint(&self) -> usize {
size_of::<i64>()
}
}

impl<'r> Decode<'r, Postgres> for Time {
fn decode(value: PgValueRef<'r>) -> Result<Self, BoxDynError> {
Ok(match value.format() {
PgValueFormat::Binary => {
// TIME is encoded as the microseconds since midnight
let us: i64 = Decode::<Postgres>::decode(value)?;
let time = jiff::civil::Time::midnight()
.checked_add(SignedDuration::from_micros(us))?;
time.to_sqlx()
}
PgValueFormat::Text => {
let s = value.as_str()?;
let time = jiff::civil::Time::strptime("%H:%M:%S%.f", s)?;
time.to_sqlx()
}
})
}
}
Loading
Loading