From 8c0dd4dc94d720fde5f67d10e9780716a1199de7 Mon Sep 17 00:00:00 2001 From: Jonathan Boyle Date: Fri, 23 Feb 2024 10:59:32 +0000 Subject: [PATCH 1/9] Change debug asserts to errors to allow for better diagnostics --- diesel/src/pg/types/floats/mod.rs | 38 +++++++++++++++---------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/diesel/src/pg/types/floats/mod.rs b/diesel/src/pg/types/floats/mod.rs index c5ee3bfb9b2f..005cffecf3f4 100644 --- a/diesel/src/pg/types/floats/mod.rs +++ b/diesel/src/pg/types/floats/mod.rs @@ -117,16 +117,15 @@ 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 Err(Box::new("Received less than 4 bytes while decoding an f32. Was a numeric accidentally marked as float?")); + } + + if (bytes.len() > 4) { + return Err(Box::new("Received more than 4 bytes while decoding an f32. Was a double accidentally marked as float?")); + } + bytes .read_f32::() .map_err(|e| Box::new(e) as Box) @@ -137,16 +136,15 @@ 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 Err(Box::new("Received less than 8 bytes while decoding an f64. Was a float accidentally marked as double?")); + } + + if bytes.len() > 8 { + return Err(Box::new("Received more than 8 bytes while decoding an f64. Was a numeric accidentally marked as double?")); + } + bytes .read_f64::() .map_err(|e| Box::new(e) as Box) From 5af2c35f5b7077b52068023abe22613dc78e5768 Mon Sep 17 00:00:00 2001 From: Jonathan Boyle Date: Fri, 23 Feb 2024 14:49:08 +0000 Subject: [PATCH 2/9] Fix error output of float conversion, and map to field name. --- diesel/src/deserialize.rs | 1 + diesel/src/pg/types/floats/mod.rs | 26 ++++++++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 920c318e9f7a..054e480b4360 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -541,6 +541,7 @@ where let field = row.get(0).ok_or(crate::result::UnexpectedEndOfRow)?; T::from_nullable_sql(field.value()) + .map_err(|e| format!("Error for field {}: {:?}", field.field_name().unwrap(), e).into()) } } diff --git a/diesel/src/pg/types/floats/mod.rs b/diesel/src/pg/types/floats/mod.rs index 005cffecf3f4..f7d247c12174 100644 --- a/diesel/src/pg/types/floats/mod.rs +++ b/diesel/src/pg/types/floats/mod.rs @@ -118,12 +118,15 @@ impl FromSql for f32 { fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - if (bytes.len() < 4) { - return Err(Box::new("Received less than 4 bytes while decoding an f32. Was a numeric accidentally marked as float?")); - } + #[cfg(debug_assertions)] + { + 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 Err(Box::new("Received more than 4 bytes while decoding an f32. Was a double accidentally marked as float?")); + 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 @@ -137,12 +140,15 @@ impl FromSql for f64 { fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - if bytes.len() < 8 { - return Err(Box::new("Received less than 8 bytes while decoding an f64. Was a float accidentally marked as double?")); - } + #[cfg(debug_assertions)] + { + 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 Err(Box::new("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 more than 8 bytes while decoding an f64. Was a numeric accidentally marked as double?".into()); + } } bytes From 9ba20f75f7378cad0159405ecf268e8ccda28028 Mon Sep 17 00:00:00 2001 From: Jonathan Boyle Date: Fri, 23 Feb 2024 14:50:37 +0000 Subject: [PATCH 3/9] Clarified error message from mapping --- diesel/src/deserialize.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 054e480b4360..2afbfa93ac37 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -540,8 +540,9 @@ where use crate::row::Field; let field = row.get(0).ok_or(crate::result::UnexpectedEndOfRow)?; - T::from_nullable_sql(field.value()) - .map_err(|e| format!("Error for field {}: {:?}", field.field_name().unwrap(), e).into()) + T::from_nullable_sql(field.value()).map_err(|e| { + format!("Error for field '{}': {:?}", field.field_name().unwrap(), e).into() + }) } } From f5682c843662cfa6ccbb4a604adc5a973083048f Mon Sep 17 00:00:00 2001 From: Jonathan Boyle Date: Fri, 23 Feb 2024 17:03:54 +0000 Subject: [PATCH 4/9] Removed error messages from debug assertions --- diesel/src/pg/types/floats/mod.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/diesel/src/pg/types/floats/mod.rs b/diesel/src/pg/types/floats/mod.rs index f7d247c12174..3fbf89f99f44 100644 --- a/diesel/src/pg/types/floats/mod.rs +++ b/diesel/src/pg/types/floats/mod.rs @@ -118,15 +118,12 @@ impl FromSql for f32 { fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - #[cfg(debug_assertions)] - { - 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 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()); - } + 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 @@ -140,15 +137,12 @@ impl FromSql for f64 { fn from_sql(value: PgValue<'_>) -> deserialize::Result { let mut bytes = value.as_bytes(); - #[cfg(debug_assertions)] - { - 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 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()); - } + 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 From e412d5f727a016c5601f9ce44758a8db131f45d7 Mon Sep 17 00:00:00 2001 From: Jonathan Boyle Date: Fri, 23 Feb 2024 17:16:40 +0000 Subject: [PATCH 5/9] Convert passed error into specific DeserializeFieldError --- diesel/src/deserialize.rs | 10 +++++++--- diesel/src/result.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 2afbfa93ac37..8587cf55157f 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -5,6 +5,7 @@ use std::result; use crate::backend::Backend; use crate::expression::select_by::SelectBy; +use crate::result::DeserializeFieldError; use crate::row::{NamedRow, Row}; use crate::sql_types::{SingleValue, SqlType, Untyped}; use crate::Selectable; @@ -540,9 +541,12 @@ where use crate::row::Field; let field = row.get(0).ok_or(crate::result::UnexpectedEndOfRow)?; - T::from_nullable_sql(field.value()).map_err(|e| { - format!("Error for field '{}': {:?}", field.field_name().unwrap(), e).into() - }) + Ok(T::from_nullable_sql(field.value()).map_err(|e| { + Box::new(DeserializeFieldError { + field_name: field.field_name().map(|s| s.to_string()), + error: e, + }) + })?) } } diff --git a/diesel/src/result.rs b/diesel/src/result.rs index a88796f78fd1..1e6c8e6293d3 100644 --- a/diesel/src/result.rs +++ b/diesel/src/result.rs @@ -455,3 +455,32 @@ impl fmt::Display for EmptyChangeset { } impl StdError for EmptyChangeset {} + +/// An error occurred while deserializing a field +#[derive(Debug)] +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 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) + } + } +} From f4dfc36c8ebbda4bcaa1251403e73a25d4c4ca5a Mon Sep 17 00:00:00 2001 From: Jonathan Boyle Date: Tue, 27 Feb 2024 10:08:45 +0000 Subject: [PATCH 6/9] Allow UnexpectedNullError to pass through, and make DeserializeFieldError non_exhaustive --- diesel/src/deserialize.rs | 4 ++++ diesel/src/result.rs | 1 + 2 files changed, 5 insertions(+) diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 8587cf55157f..a2a130786480 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -542,6 +542,10 @@ where let field = row.get(0).ok_or(crate::result::UnexpectedEndOfRow)?; Ok(T::from_nullable_sql(field.value()).map_err(|e| { + if e.is::() { + return e; + } + Box::new(DeserializeFieldError { field_name: field.field_name().map(|s| s.to_string()), error: e, diff --git a/diesel/src/result.rs b/diesel/src/result.rs index 1e6c8e6293d3..9ee94d8a6bba 100644 --- a/diesel/src/result.rs +++ b/diesel/src/result.rs @@ -458,6 +458,7 @@ 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, From 1544597b728312b19b373279b45c75c635873132 Mon Sep 17 00:00:00 2001 From: Jonathan Boyle Date: Tue, 27 Feb 2024 15:24:38 +0000 Subject: [PATCH 7/9] satisfy clippy --- diesel/src/deserialize.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index a2a130786480..3525cb262d6f 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -541,7 +541,7 @@ where use crate::row::Field; let field = row.get(0).ok_or(crate::result::UnexpectedEndOfRow)?; - Ok(T::from_nullable_sql(field.value()).map_err(|e| { + T::from_nullable_sql(field.value()).map_err(|e| { if e.is::() { return e; } @@ -550,7 +550,7 @@ where field_name: field.field_name().map(|s| s.to_string()), error: e, }) - })?) + }) } } From 3e10a04cfec184ee5c935793d8b6556107de85ff Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 8 Mar 2024 10:24:19 +0100 Subject: [PATCH 8/9] Address review comments * Add tests * Add changelog entry * Adjust impls for integers as well --- CHANGELOG.md | 1 + diesel/src/pg/types/floats/mod.rs | 24 +++++-- diesel/src/pg/types/integers.rs | 76 ++++++++++++-------- diesel_tests/tests/types.rs | 114 +++++++++++++++++++++++++++++- 4 files changed, 179 insertions(+), 36 deletions(-) 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/pg/types/floats/mod.rs b/diesel/src/pg/types/floats/mod.rs index 3fbf89f99f44..b35ee0467966 100644 --- a/diesel/src/pg/types/floats/mod.rs +++ b/diesel/src/pg/types/floats/mod.rs @@ -119,11 +119,19 @@ impl FromSql for f32 { let mut bytes = value.as_bytes(); 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()); + 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()); + return deserialize::Result::Err( + "Received more than 4 bytes while decoding an f32. \ + Was a double accidentally marked as float?" + .into(), + ); } bytes @@ -138,11 +146,19 @@ impl FromSql for f64 { let mut bytes = value.as_bytes(); 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()); + 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()); + return deserialize::Result::Err( + "Received more than 8 bytes while decoding an f64. \ + Was a numeric accidentally marked as double?" + .into(), + ); } bytes diff --git a/diesel/src/pg/types/integers.rs b/diesel/src/pg/types/integers.rs index 4438c3c25699..b1a56b0a3a6d 100644 --- a/diesel/src/pg/types/integers.rs +++ b/diesel/src/pg/types/integers.rs @@ -25,17 +25,21 @@ impl ToSql for u32 { impl FromSql for i16 { 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 deserialize::Result::Err( + "Received less than 2 bytes while decoding an i16. \ + Was an expression of a different type accidentally marked as SmallInt?" + .into(), + ); + } + + if bytes.len() > 2 { + return deserialize::Result::Err( + "Received more than 2 bytes while decoding an i16. \ + Was an Integer expression accidentally marked as SmallInt?" + .into(), + ); + } bytes .read_i16::() .map_err(|e| Box::new(e) as Box<_>) @@ -46,16 +50,21 @@ impl FromSql for i16 { impl FromSql for i32 { 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 deserialize::Result::Err( + "Received less than 4 bytes while decoding an i32. \ + Was an SmallInt expression accidentally marked as Integer?" + .into(), + ); + } + + if bytes.len() > 4 { + return deserialize::Result::Err( + "Received more than 4 bytes while decoding an i32. \ + Was an BigInt expression accidentally marked as Integer?" + .into(), + ); + } bytes .read_i32::() .map_err(|e| Box::new(e) as Box<_>) @@ -66,16 +75,21 @@ impl FromSql for i32 { impl FromSql for i64 { 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 deserialize::Result::Err( + "Received less than 8 bytes while decoding an i64. \ + Was an Integer expression accidentally marked as BigInt?" + .into(), + ); + } + + if bytes.len() > 8 { + return deserialize::Result::Err( + "Received more than 8 bytes while decoding an i64. \ + Was an expression of a different type expression accidentally marked as BigInt?" + .into(), + ); + } bytes .read_i64::() .map_err(|e| Box::new(e) as Box<_>) 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?" + ); +} From c19ace787f9a3b9460eb34c40b473741afbbc99d Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 13 Mar 2024 10:48:11 +0100 Subject: [PATCH 9/9] Minor optimisations * Mark the error branches as cold to help the compiler optimising for the other case * Mark some functions as inline Both changes have been benchmarked and have shown minor performance improvements --- diesel/src/deserialize.rs | 10 +++----- diesel/src/pg/connection/result.rs | 1 + diesel/src/pg/types/integers.rs | 37 ++++++++++++++++-------------- diesel/src/result.rs | 14 +++++++++++ diesel_bench/Cargo.toml | 1 + 5 files changed, 39 insertions(+), 24 deletions(-) diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 3525cb262d6f..217bc6742755 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -5,7 +5,6 @@ use std::result; use crate::backend::Backend; use crate::expression::select_by::SelectBy; -use crate::result::DeserializeFieldError; use crate::row::{NamedRow, Row}; use crate::sql_types::{SingleValue, SqlType, Untyped}; use crate::Selectable; @@ -543,13 +542,10 @@ where let field = row.get(0).ok_or(crate::result::UnexpectedEndOfRow)?; T::from_nullable_sql(field.value()).map_err(|e| { if e.is::() { - return e; + e + } else { + Box::new(crate::result::DeserializeFieldError::new(field, e)) } - - Box::new(DeserializeFieldError { - field_name: field.field_name().map(|s| s.to_string()), - error: 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/integers.rs b/diesel/src/pg/types/integers.rs index b1a56b0a3a6d..4b7ab7a9bc19 100644 --- a/diesel/src/pg/types/integers.rs +++ b/diesel/src/pg/types/integers.rs @@ -23,21 +23,20 @@ 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(); if bytes.len() < 2 { - return deserialize::Result::Err( + return emit_size_error( "Received less than 2 bytes while decoding an i16. \ - Was an expression of a different type accidentally marked as SmallInt?" - .into(), + Was an expression of a different type accidentally marked as SmallInt?", ); } if bytes.len() > 2 { - return deserialize::Result::Err( + return emit_size_error( "Received more than 2 bytes while decoding an i16. \ - Was an Integer expression accidentally marked as SmallInt?" - .into(), + Was an Integer expression accidentally marked as SmallInt?", ); } bytes @@ -48,21 +47,20 @@ 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(); if bytes.len() < 4 { - return deserialize::Result::Err( + return emit_size_error( "Received less than 4 bytes while decoding an i32. \ - Was an SmallInt expression accidentally marked as Integer?" - .into(), + Was an SmallInt expression accidentally marked as Integer?", ); } if bytes.len() > 4 { - return deserialize::Result::Err( + return emit_size_error( "Received more than 4 bytes while decoding an i32. \ - Was an BigInt expression accidentally marked as Integer?" - .into(), + Was an BigInt expression accidentally marked as Integer?", ); } bytes @@ -71,23 +69,28 @@ impl FromSql for i32 { } } +#[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(); if bytes.len() < 8 { - return deserialize::Result::Err( + return emit_size_error( "Received less than 8 bytes while decoding an i64. \ - Was an Integer expression accidentally marked as BigInt?" - .into(), + Was an Integer expression accidentally marked as BigInt?", ); } if bytes.len() > 8 { - return deserialize::Result::Err( + 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?" - .into(), ); } bytes diff --git a/diesel/src/result.rs b/diesel/src/result.rs index 9ee94d8a6bba..acf12423b068 100644 --- a/diesel/src/result.rs +++ b/diesel/src/result.rs @@ -466,6 +466,20 @@ pub struct DeserializeFieldError { 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) 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]