diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bf5be124b65..703c2064adb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Increasing the minimal supported Rust version will always be coupled at least wi * The minimal officially supported rustc version is now 1.78.0 * Deprecated `sql_function!` in favour of `define_sql_function!` which provides compatibility with `#[dsl::auto_type]` +* Deserialization error messages now contain information about the field that failed to deserialize ## [2.1.0] 2023-05-26 diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 920c318e9f7a..217bc6742755 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -540,7 +540,13 @@ where use crate::row::Field; let field = row.get(0).ok_or(crate::result::UnexpectedEndOfRow)?; - T::from_nullable_sql(field.value()) + T::from_nullable_sql(field.value()).map_err(|e| { + if e.is::() { + e + } else { + Box::new(crate::result::DeserializeFieldError::new(field, e)) + } + }) } } diff --git a/diesel/src/pg/connection/result.rs b/diesel/src/pg/connection/result.rs index 4b00f718dc37..b4f501669f3d 100644 --- a/diesel/src/pg/connection/result.rs +++ b/diesel/src/pg/connection/result.rs @@ -148,6 +148,7 @@ impl PgResult { ) } + #[inline(always)] // benchmarks indicate a ~1.7% improvement in instruction count for this pub(super) fn column_name(&self, col_idx: usize) -> Option<&str> { self.column_name_map .get_or_init(|| { diff --git a/diesel/src/pg/types/floats/mod.rs b/diesel/src/pg/types/floats/mod.rs index c5ee3bfb9b2f..b35ee0467966 100644 --- a/diesel/src/pg/types/floats/mod.rs +++ b/diesel/src/pg/types/floats/mod.rs @@ -117,16 +117,23 @@ impl ToSql for PgNumeric { impl FromSql for f32 { fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - debug_assert!( - bytes.len() <= 4, - "Received more than 4 bytes while decoding \ - an f32. Was a double accidentally marked as float?" - ); - debug_assert!( - bytes.len() >= 4, - "Received less than 4 bytes while decoding \ - an f32." - ); + + if bytes.len() < 4 { + return deserialize::Result::Err( + "Received less than 4 bytes while decoding an f32. \ + Was a numeric accidentally marked as float?" + .into(), + ); + } + + if bytes.len() > 4 { + return deserialize::Result::Err( + "Received more than 4 bytes while decoding an f32. \ + Was a double accidentally marked as float?" + .into(), + ); + } + bytes .read_f32::() .map_err(|e| Box::new(e) as Box) @@ -137,16 +144,23 @@ impl FromSql for f32 { impl FromSql for f64 { fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - debug_assert!( - bytes.len() <= 8, - "Received less than 8 bytes while decoding \ - an f64. Was a float accidentally marked as double?" - ); - debug_assert!( - bytes.len() >= 8, - "Received more than 8 bytes while decoding \ - an f64. Was a numeric accidentally marked as double?" - ); + + if bytes.len() < 8 { + return deserialize::Result::Err( + "Received less than 8 bytes while decoding an f64. \ + Was a float accidentally marked as double?" + .into(), + ); + } + + if bytes.len() > 8 { + return deserialize::Result::Err( + "Received more than 8 bytes while decoding an f64. \ + Was a numeric accidentally marked as double?" + .into(), + ); + } + bytes .read_f64::() .map_err(|e| Box::new(e) as Box) diff --git a/diesel/src/pg/types/integers.rs b/diesel/src/pg/types/integers.rs index 4438c3c25699..4b7ab7a9bc19 100644 --- a/diesel/src/pg/types/integers.rs +++ b/diesel/src/pg/types/integers.rs @@ -23,19 +23,22 @@ impl ToSql for u32 { #[cfg(feature = "postgres_backend")] impl FromSql for i16 { + #[inline(always)] fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - debug_assert!( - bytes.len() <= 2, - "Received more than 2 bytes decoding i16. \ - Was an Integer expression accidentally identified as SmallInt?" - ); - debug_assert!( - bytes.len() >= 2, - "Received fewer than 2 bytes decoding i16. \ - Was an expression of a different type accidentally identified \ - as SmallInt?" - ); + if bytes.len() < 2 { + return emit_size_error( + "Received less than 2 bytes while decoding an i16. \ + Was an expression of a different type accidentally marked as SmallInt?", + ); + } + + if bytes.len() > 2 { + return emit_size_error( + "Received more than 2 bytes while decoding an i16. \ + Was an Integer expression accidentally marked as SmallInt?", + ); + } bytes .read_i16::() .map_err(|e| Box::new(e) as Box<_>) @@ -44,38 +47,52 @@ impl FromSql for i16 { #[cfg(feature = "postgres_backend")] impl FromSql for i32 { + #[inline(always)] fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - debug_assert!( - bytes.len() <= 4, - "Received more than 4 bytes decoding i32. \ - Was a BigInt expression accidentally identified as Integer?" - ); - debug_assert!( - bytes.len() >= 4, - "Received fewer than 4 bytes decoding i32. \ - Was a SmallInt expression accidentally identified as Integer?" - ); + if bytes.len() < 4 { + return emit_size_error( + "Received less than 4 bytes while decoding an i32. \ + Was an SmallInt expression accidentally marked as Integer?", + ); + } + + if bytes.len() > 4 { + return emit_size_error( + "Received more than 4 bytes while decoding an i32. \ + Was an BigInt expression accidentally marked as Integer?", + ); + } bytes .read_i32::() .map_err(|e| Box::new(e) as Box<_>) } } +#[cold] +#[inline(never)] +fn emit_size_error(var_name: &str) -> deserialize::Result { + deserialize::Result::Err(var_name.into()) +} + #[cfg(feature = "postgres_backend")] impl FromSql for i64 { + #[inline(always)] fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - debug_assert!( - bytes.len() <= 8, - "Received more than 8 bytes decoding i64. \ - Was an expression of a different type misidentified as BigInt?" - ); - debug_assert!( - bytes.len() >= 8, - "Received fewer than 8 bytes decoding i64. \ - Was an Integer expression misidentified as BigInt?" - ); + if bytes.len() < 8 { + return emit_size_error( + "Received less than 8 bytes while decoding an i64. \ + Was an Integer expression accidentally marked as BigInt?", + ); + } + + if bytes.len() > 8 { + return emit_size_error( + "Received more than 8 bytes while decoding an i64. \ + Was an expression of a different type expression accidentally marked as BigInt?" + ); + } bytes .read_i64::() .map_err(|e| Box::new(e) as Box<_>) diff --git a/diesel/src/result.rs b/diesel/src/result.rs index a88796f78fd1..acf12423b068 100644 --- a/diesel/src/result.rs +++ b/diesel/src/result.rs @@ -455,3 +455,47 @@ impl fmt::Display for EmptyChangeset { } impl StdError for EmptyChangeset {} + +/// An error occurred while deserializing a field +#[derive(Debug)] +#[non_exhaustive] +pub struct DeserializeFieldError { + /// The name of the field that failed to deserialize + pub field_name: Option, + /// The error that occurred while deserializing the field + pub error: Box, +} + +impl DeserializeFieldError { + #[cold] + pub(crate) fn new<'a, F, DB>(field: F, error: Box) -> Self + where + DB: crate::backend::Backend, + F: crate::row::Field<'a, DB>, + { + DeserializeFieldError { + field_name: field.field_name().map(|s| s.to_string()), + error, + } + } +} + +impl StdError for DeserializeFieldError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + Some(&*self.error) + } +} + +impl fmt::Display for DeserializeFieldError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(ref field_name) = self.field_name { + write!( + f, + "Error deserializing field '{}': {}", + field_name, self.error + ) + } else { + write!(f, "Error deserializing field: {}", self.error) + } + } +} diff --git a/diesel_bench/Cargo.toml b/diesel_bench/Cargo.toml index 990d2d1c93ad..f788892eb3af 100644 --- a/diesel_bench/Cargo.toml +++ b/diesel_bench/Cargo.toml @@ -71,6 +71,7 @@ fast_run = [] [profile.release] lto = true +debug = true codegen-units = 1 [patch.crates-io] diff --git a/diesel_tests/tests/types.rs b/diesel_tests/tests/types.rs index 6e1e0c739b50..e1cfd0416d8e 100644 --- a/diesel_tests/tests/types.rs +++ b/diesel_tests/tests/types.rs @@ -1388,7 +1388,7 @@ where #[cfg(feature = "postgres")] #[test] -#[should_panic(expected = "Received more than 4 bytes decoding i32")] +#[should_panic(expected = "Received more than 4 bytes while decoding an i32")] fn debug_check_catches_reading_bigint_as_i32_when_using_raw_sql() { use diesel::dsl::sql; use diesel::sql_types::Integer; @@ -1574,3 +1574,115 @@ fn citext_fields() { assert_eq!(lowercase_in_db, Some("lowercase_value".to_string())); } + +#[test] +#[cfg(feature = "postgres")] +fn deserialize_wrong_primitive_gives_good_error() { + let conn = &mut connection(); + + diesel::sql_query( + "CREATE TABLE test_table(\ + bool BOOLEAN, + small SMALLINT, \ + int INTEGER, \ + big BIGINT, \ + float FLOAT4, \ + double FLOAT8, + text TEXT)", + ) + .execute(conn) + .unwrap(); + diesel::sql_query("INSERT INTO test_table VALUES('t', 1, 1, 1, 1, 1, 'long text long text')") + .execute(conn) + .unwrap(); + + let res = diesel::dsl::sql::("SELECT bool FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'bool': \ + Received less than 2 bytes while decoding an i16. \ + Was an expression of a different type accidentally marked as SmallInt?" + ); + + let res = diesel::dsl::sql::("SELECT int FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'int': \ + Received more than 2 bytes while decoding an i16. \ + Was an Integer expression accidentally marked as SmallInt?" + ); + + let res = diesel::dsl::sql::("SELECT small FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'small': \ + Received less than 4 bytes while decoding an i32. \ + Was an SmallInt expression accidentally marked as Integer?" + ); + + let res = diesel::dsl::sql::("SELECT big FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'big': \ + Received more than 4 bytes while decoding an i32. \ + Was an BigInt expression accidentally marked as Integer?" + ); + + let res = diesel::dsl::sql::("SELECT int FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'int': \ + Received less than 8 bytes while decoding an i64. \ + Was an Integer expression accidentally marked as BigInt?" + ); + + let res = diesel::dsl::sql::("SELECT text FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'text': \ + Received more than 8 bytes while decoding an i64. \ + Was an expression of a different type expression accidentally marked as BigInt?" + ); + + let res = diesel::dsl::sql::("SELECT small FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'small': \ + Received less than 4 bytes while decoding an f32. \ + Was a numeric accidentally marked as float?" + ); + + let res = diesel::dsl::sql::("SELECT double FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'double': \ + Received more than 4 bytes while decoding an f32. \ + Was a double accidentally marked as float?" + ); + + let res = diesel::dsl::sql::("SELECT float FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'float': \ + Received less than 8 bytes while decoding an f64. \ + Was a float accidentally marked as double?" + ); + + let res = diesel::dsl::sql::("SELECT text FROM test_table").get_result::(conn); + assert!(res.is_err()); + assert_eq!( + res.unwrap_err().to_string(), + "Error deserializing field 'text': \ + Received more than 8 bytes while decoding an f64. \ + Was a numeric accidentally marked as double?" + ); +}