From d285e5886df32b3bfbdb47dd831452c2e902833b Mon Sep 17 00:00:00 2001 From: Moritz Pflanzer Date: Wed, 9 Oct 2024 13:00:22 +0200 Subject: [PATCH] Fix deserialization of empty PostgreSQL ranges In diesel empty ranges are represented as: ``` ( Bound::Excluded(T::default_value()), Bound::Excluded(T::default_value()), ) ``` --- diesel/src/deserialize.rs | 23 +++++++++++++ diesel/src/pg/types/date_and_time/chrono.rs | 30 ++++++++++++++++- diesel/src/pg/types/date_and_time/time.rs | 23 ++++++++++++- diesel/src/pg/types/floats/mod.rs | 12 +++++-- diesel/src/pg/types/integers.rs | 16 ++++++++- diesel/src/pg/types/multirange.rs | 4 +-- diesel/src/pg/types/numeric.rs | 9 ++++- diesel/src/pg/types/ranges.rs | 14 +++++--- diesel_tests/tests/types.rs | 37 ++++++++++++++++++++- 9 files changed, 154 insertions(+), 14 deletions(-) diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 217bc6742755..05ab4b388022 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -578,3 +578,26 @@ where { const FIELD_COUNT: usize = ::SIZE; } + +/// A helper trait for giving a type a useful default value. +/// +/// This is needed for types that can be used as range to represent the empty range as +/// (Bound::Excluded(DEFAULT), Bound::Excluded(DEFAULT)). +/// When possible, implementations of this trait should fall back to using std::default::Default. +#[allow(dead_code)] +pub(crate) trait Defaultable { + /// Returns the "default value" for a type. + fn default_value() -> Self; +} + +// We cannot have this impl because rustc +// then complains in third party crates that +// diesel may implement `Default`in the future. +// If we get negative trait impls at some point in time +// it should be possible to make this work. +//// Defaultable for types that has Default +//impl Defaultable for T { +// fn default_value() -> Self { +// T::default() +// } +//} diff --git a/diesel/src/pg/types/date_and_time/chrono.rs b/diesel/src/pg/types/date_and_time/chrono.rs index 756e0bf7814f..34960ac227bd 100644 --- a/diesel/src/pg/types/date_and_time/chrono.rs +++ b/diesel/src/pg/types/date_and_time/chrono.rs @@ -5,7 +5,7 @@ extern crate chrono; use self::chrono::{DateTime, Duration, Local, NaiveDate, NaiveDateTime, NaiveTime, TimeZone, Utc}; use super::{PgDate, PgInterval, PgTime, PgTimestamp}; -use crate::deserialize::{self, FromSql}; +use crate::deserialize::{self, Defaultable, FromSql}; use crate::pg::{Pg, PgValue}; use crate::serialize::{self, Output, ToSql}; use crate::sql_types::{Date, Interval, Time, Timestamp, Timestamptz}; @@ -61,6 +61,13 @@ impl ToSql for NaiveDateTime { } } +#[cfg(all(feature = "chrono", feature = "postgres_backend"))] +impl Defaultable for NaiveDateTime { + fn default_value() -> Self { + Self::default() + } +} + #[cfg(all(feature = "chrono", feature = "postgres_backend"))] impl FromSql for DateTime { fn from_sql(bytes: PgValue<'_>) -> deserialize::Result { @@ -69,6 +76,13 @@ impl FromSql for DateTime { } } +#[cfg(all(feature = "chrono", feature = "postgres_backend"))] +impl Defaultable for DateTime { + fn default_value() -> Self { + Self::default() + } +} + #[cfg(all(feature = "chrono", feature = "postgres_backend"))] impl FromSql for DateTime { fn from_sql(bytes: PgValue<'_>) -> deserialize::Result { @@ -77,6 +91,13 @@ impl FromSql for DateTime { } } +#[cfg(all(feature = "chrono", feature = "postgres_backend"))] +impl Defaultable for DateTime { + fn default_value() -> Self { + Self::default() + } +} + #[cfg(all(feature = "chrono", feature = "postgres_backend"))] impl ToSql for DateTime { fn to_sql<'b>(&'b self, out: &mut Output<'b, '_, Pg>) -> serialize::Result { @@ -139,6 +160,13 @@ impl FromSql for NaiveDate { } } +#[cfg(all(feature = "chrono", feature = "postgres_backend"))] +impl Defaultable for NaiveDate { + fn default_value() -> Self { + Self::default() + } +} + const DAYS_PER_MONTH: i32 = 30; const SECONDS_PER_DAY: i64 = 60 * 60 * 24; const MICROSECONDS_PER_SECOND: i64 = 1_000_000; diff --git a/diesel/src/pg/types/date_and_time/time.rs b/diesel/src/pg/types/date_and_time/time.rs index d1d1affab937..eca92d901a5d 100644 --- a/diesel/src/pg/types/date_and_time/time.rs +++ b/diesel/src/pg/types/date_and_time/time.rs @@ -9,7 +9,7 @@ use self::time::{ }; use super::{PgDate, PgTime, PgTimestamp}; -use crate::deserialize::{self, FromSql}; +use crate::deserialize::{self, Defaultable, FromSql}; use crate::pg::{Pg, PgValue}; use crate::serialize::{self, Output, ToSql}; use crate::sql_types::{Date, Time, Timestamp, Timestamptz}; @@ -57,6 +57,13 @@ impl ToSql for PrimitiveDateTime { } } +#[cfg(all(feature = "time", feature = "postgres_backend"))] +impl Defaultable for PrimitiveDateTime { + fn default_value() -> Self { + PG_EPOCH + } +} + #[cfg(all(feature = "time", feature = "postgres_backend"))] impl FromSql for OffsetDateTime { fn from_sql(bytes: PgValue<'_>) -> deserialize::Result { @@ -74,6 +81,13 @@ impl ToSql for OffsetDateTime { } } +#[cfg(all(feature = "time", feature = "postgres_backend"))] +impl Defaultable for OffsetDateTime { + fn default_value() -> Self { + datetime!(2000-01-01 0:00:00 UTC) + } +} + #[cfg(all(feature = "time", feature = "postgres_backend"))] impl ToSql for NaiveTime { fn to_sql<'b>(&'b self, out: &mut Output<'b, '_, Pg>) -> serialize::Result { @@ -119,6 +133,13 @@ impl FromSql for NaiveDate { } } +#[cfg(all(feature = "time", feature = "postgres_backend"))] +impl Defaultable for NaiveDate { + fn default_value() -> Self { + PG_EPOCH_DATE + } +} + #[cfg(test)] mod tests { extern crate dotenvy; diff --git a/diesel/src/pg/types/floats/mod.rs b/diesel/src/pg/types/floats/mod.rs index da6256937eff..8a927d37e1e9 100644 --- a/diesel/src/pg/types/floats/mod.rs +++ b/diesel/src/pg/types/floats/mod.rs @@ -1,4 +1,4 @@ -use crate::deserialize::{self, FromSql, FromSqlRow}; +use crate::deserialize::{self, Defaultable, FromSql, FromSqlRow}; use crate::expression::AsExpression; use crate::pg::{Pg, PgValue}; use crate::serialize::{self, IsNull, Output, ToSql}; @@ -9,7 +9,7 @@ use std::error::Error; #[cfg(feature = "quickcheck")] mod quickcheck_impls; -#[derive(Debug, Clone, PartialEq, Eq, AsExpression, FromSqlRow)] +#[derive(Debug, Default, Clone, PartialEq, Eq, AsExpression, FromSqlRow)] #[diesel(sql_type = sql_types::Numeric)] /// Represents a NUMERIC value, closely mirroring the PG wire protocol /// representation @@ -33,6 +33,7 @@ pub enum PgNumeric { digits: Vec, }, /// Not a number + #[default] NaN, } @@ -113,6 +114,13 @@ impl ToSql for PgNumeric { } } +#[cfg(feature = "postgres_backend")] +impl Defaultable for PgNumeric { + fn default_value() -> Self { + Self::default() + } +} + #[cfg(feature = "postgres_backend")] impl FromSql for f32 { fn from_sql(value: PgValue<'_>) -> deserialize::Result { diff --git a/diesel/src/pg/types/integers.rs b/diesel/src/pg/types/integers.rs index 4b7ab7a9bc19..c9af30caa66a 100644 --- a/diesel/src/pg/types/integers.rs +++ b/diesel/src/pg/types/integers.rs @@ -1,4 +1,4 @@ -use crate::deserialize::{self, FromSql}; +use crate::deserialize::{self, Defaultable, FromSql}; use crate::pg::{Pg, PgValue}; use crate::serialize::{self, IsNull, Output, ToSql}; use crate::sql_types; @@ -126,6 +126,20 @@ impl ToSql for i64 { } } +#[cfg(feature = "postgres_backend")] +impl Defaultable for i32 { + fn default_value() -> Self { + Self::default() + } +} + +#[cfg(feature = "postgres_backend")] +impl Defaultable for i64 { + fn default_value() -> Self { + Self::default() + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/diesel/src/pg/types/multirange.rs b/diesel/src/pg/types/multirange.rs index 0190e5533d8c..e1fa51f46d72 100644 --- a/diesel/src/pg/types/multirange.rs +++ b/diesel/src/pg/types/multirange.rs @@ -2,7 +2,7 @@ use byteorder::{NetworkEndian, ReadBytesExt, WriteBytesExt}; use std::io::Write; use std::ops::Bound; -use crate::deserialize::{self, FromSql}; +use crate::deserialize::{self, Defaultable, FromSql}; use crate::expression::bound::Bound as SqlBound; use crate::expression::AsExpression; use crate::pg::{Pg, PgTypeMetadata, PgValue}; @@ -68,7 +68,7 @@ multirange_as_expressions!(std::ops::RangeTo); #[cfg(feature = "postgres_backend")] impl FromSql, Pg> for Vec<(Bound, Bound)> where - T: FromSql, + T: FromSql + Defaultable, { fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); diff --git a/diesel/src/pg/types/numeric.rs b/diesel/src/pg/types/numeric.rs index 18a7f9778def..c0496b90e1db 100644 --- a/diesel/src/pg/types/numeric.rs +++ b/diesel/src/pg/types/numeric.rs @@ -10,7 +10,7 @@ mod bigdecimal { use self::num_integer::Integer; use self::num_traits::{Signed, ToPrimitive, Zero}; - use crate::deserialize::{self, FromSql}; + use crate::deserialize::{self, Defaultable, FromSql}; use crate::pg::data_types::PgNumeric; use crate::pg::{Pg, PgValue}; use crate::serialize::{self, Output, ToSql}; @@ -169,6 +169,13 @@ mod bigdecimal { } } + #[cfg(all(feature = "postgres_backend", feature = "numeric"))] + impl Defaultable for BigDecimal { + fn default_value() -> Self { + Self::default() + } + } + #[cfg(test)] mod tests { use super::*; diff --git a/diesel/src/pg/types/ranges.rs b/diesel/src/pg/types/ranges.rs index 10c439b4b850..16d053269863 100644 --- a/diesel/src/pg/types/ranges.rs +++ b/diesel/src/pg/types/ranges.rs @@ -3,7 +3,7 @@ use std::collections::Bound; use std::error::Error; use std::io::Write; -use crate::deserialize::{self, FromSql, Queryable}; +use crate::deserialize::{self, Defaultable, FromSql, Queryable}; use crate::expression::bound::Bound as SqlBound; use crate::expression::AsExpression; use crate::pg::{Pg, PgTypeMetadata, PgValue}; @@ -74,7 +74,7 @@ range_as_expression!(&'a std::ops::RangeTo; Nullable>); #[cfg(feature = "postgres_backend")] impl FromSql, Pg> for (Bound, Bound) where - T: FromSql, + T: FromSql + Defaultable, { fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); @@ -82,7 +82,9 @@ where let mut lower_bound = Bound::Unbounded; let mut upper_bound = Bound::Unbounded; - if !flags.contains(RangeFlags::LB_INF) { + if flags.contains(RangeFlags::EMPTY) { + lower_bound = Bound::Excluded(T::default_value()); + } else if !flags.contains(RangeFlags::LB_INF) { let elem_size = bytes.read_i32::()?; let (elem_bytes, new_bytes) = bytes.split_at(elem_size.try_into()?); bytes = new_bytes; @@ -95,7 +97,9 @@ where }; } - if !flags.contains(RangeFlags::UB_INF) { + if flags.contains(RangeFlags::EMPTY) { + upper_bound = Bound::Excluded(T::default_value()); + } else if !flags.contains(RangeFlags::UB_INF) { let _size = bytes.read_i32::()?; let value = T::from_sql(PgValue::new_internal(bytes, &value))?; @@ -113,7 +117,7 @@ where #[cfg(feature = "postgres_backend")] impl Queryable, Pg> for (Bound, Bound) where - T: FromSql, + T: FromSql + Defaultable, { type Row = Self; diff --git a/diesel_tests/tests/types.rs b/diesel_tests/tests/types.rs index 8f2f2ccb3ad1..9f5d1026bf9d 100644 --- a/diesel_tests/tests/types.rs +++ b/diesel_tests/tests/types.rs @@ -1422,10 +1422,45 @@ fn test_range_from_sql() { query_single_value::, (Bound, Bound)>(query) ); - let query = "SELECT '(1,1]'::int4range"; + let query = "SELECT '[2,1)'::int4range"; assert!(sql::>(query) .load::<(Bound, Bound)>(connection) .is_err()); + + let query = "'empty'::int4range"; + let expected_value = (Bound::Excluded(0), Bound::Excluded(0)); + assert_eq!( + expected_value, + query_single_value::, (Bound, Bound)>(query) + ); + + let query = "'(1,1)'::int4range"; + let expected_value = (Bound::Excluded(0), Bound::Excluded(0)); + assert_eq!( + expected_value, + query_single_value::, (Bound, Bound)>(query) + ); + + let query = "'(1,1]'::int4range"; + let expected_value = (Bound::Excluded(0), Bound::Excluded(0)); + assert_eq!( + expected_value, + query_single_value::, (Bound, Bound)>(query) + ); + + let query = "'[1,1)'::int4range"; + let expected_value = (Bound::Excluded(0), Bound::Excluded(0)); + assert_eq!( + expected_value, + query_single_value::, (Bound, Bound)>(query) + ); + + let query = "'[1,1]'::int4range"; + let expected_value = (Bound::Included(1), Bound::Excluded(2)); + assert_eq!( + expected_value, + query_single_value::, (Bound, Bound)>(query) + ); } #[cfg(feature = "postgres")]