Merge pull request #2599 from Ten0/faillible_queryable_stillgeneric

Faillible Queryable

CHANGELOG.md

@@ -136,8 +136,15 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
   Similary, `only_tabels` and `except_tables` in `diesel.toml` are treated as regular expressions.
 
 * Now you can sort column fields by name with the `column-sorting` option. 
-It can be set to either `ordinal_position` (default) or `name`.
-This ensures stable sorting even if columns are removed and re-added.
+  It can be set to either `ordinal_position` (default) or `name`.
+  This ensures stable sorting even if columns are removed and re-added.
+
+* The `Queryable<ST,DB>` trait was updated to be made faillible, in order to properly handle
+  cases where you detect a data inconsistency between fields on deserialization
+  (that e.g. was supposed to be made impossible by DB `CHECK`s). The `build` function now
+  returns a
+  [`diesel::deserialize::Result<Self>`](https://docs.diesel.rs/master/diesel/deserialize/type.Result.html)
+  instead of a `Self`.
 
 ### Fixed
 

diesel/src/deserialize.rs

@@ -57,7 +57,7 @@ pub type Result<T> = result::Result<T, Box<dyn Error + Send + Sync>>;
 /// #
 /// # use schema::users;
 /// # use diesel::backend::{self, Backend};
-/// # use diesel::deserialize::{Queryable, FromSql};
+/// # use diesel::deserialize::{self, Queryable, FromSql};
 /// # use diesel::sql_types::Text;
 /// #
 /// struct LowercaseString(String);
@@ -75,8 +75,8 @@ pub type Result<T> = result::Result<T, Box<dyn Error + Send + Sync>>;
 /// {
 ///     type Row = String;
 ///
-///     fn build(s: String) -> Self {
-///         LowercaseString(s.to_lowercase())
+///     fn build(s: String) -> deserialize::Result<Self> {
+///         Ok(LowercaseString(s.to_lowercase()))
 ///     }
 /// }
 ///
@@ -107,7 +107,7 @@ pub type Result<T> = result::Result<T, Box<dyn Error + Send + Sync>>;
 /// # include!("doctest_setup.rs");
 /// #
 /// use schema::users;
-/// use diesel::deserialize::Queryable;
+/// use diesel::deserialize::{self, Queryable};
 ///
 /// # /*
 /// type DB = diesel::sqlite::Sqlite;
@@ -122,11 +122,11 @@ pub type Result<T> = result::Result<T, Box<dyn Error + Send + Sync>>;
 /// impl Queryable<users::SqlType, DB> for User {
 ///     type Row = (i32, String);
 ///
-///     fn build(row: Self::Row) -> Self {
-///         User {
+///     fn build(row: Self::Row) -> deserialize::Result<Self> {
+///         Ok(User {
 ///             id: row.0,
 ///             name: row.1.to_lowercase(),
-///         }
+///         })
 ///     }
 /// }
 ///
@@ -143,7 +143,7 @@ pub type Result<T> = result::Result<T, Box<dyn Error + Send + Sync>>;
 /// #     Ok(())
 /// # }
 /// ```
-pub trait Queryable<ST, DB>
+pub trait Queryable<ST, DB>: Sized
 where
     DB: Backend,
 {
@@ -153,7 +153,7 @@ where
     type Row: FromStaticSqlRow<ST, DB>;
 
     /// Construct an instance of this type
-    fn build(row: Self::Row) -> Self;
+    fn build(row: Self::Row) -> Result<Self>;
 }
 
 #[doc(inline)]
@@ -394,7 +394,7 @@ where
 {
     fn build_from_row<'a>(row: &impl Row<'a, DB>) -> Result<Self> {
         let row = <T::Row as FromStaticSqlRow<ST, DB>>::build_from_row(row)?;
-        Ok(T::build(row))
+        T::build(row)
     }
 }
 

diesel/src/pg/types/ranges.rs

@@ -99,8 +99,8 @@ where
 {
     type Row = Self;
 
-    fn build(row: Self) -> Self {
-        row
+    fn build(row: Self) -> deserialize::Result<Self> {
+        Ok(row)
     }
 }
 

diesel/src/pg/types/record.rs

@@ -73,8 +73,8 @@ macro_rules! tuple_impls {
         {
             type Row = Self;
 
-            fn build(row: Self::Row) -> Self {
-                row
+            fn build(row: Self::Row) -> deserialize::Result<Self> {
+                Ok(row)
             }
         }
 

diesel/src/type_impls/option.rs

@@ -106,8 +106,8 @@ where
 {
     type Row = Self;
 
-    fn build(row: Self::Row) -> Self {
-        row
+    fn build(row: Self::Row) -> deserialize::Result<Self> {
+        Ok(row)
     }
 }
 

diesel/src/type_impls/primitives.rs

@@ -241,8 +241,8 @@ where
 {
     type Row = Self;
 
-    fn build(row: Self::Row) -> Self {
-        row
+    fn build(row: Self::Row) -> deserialize::Result<Self> {
+        Ok(row)
     }
 }
 

diesel/src/type_impls/tuples.rs

@@ -235,8 +235,8 @@ macro_rules! tuple_impls {
             {
                 type Row = Self;
 
-                fn build(row: Self::Row) -> Self {
-                    row
+                fn build(row: Self::Row) -> deserialize::Result<Self> {
+                    Ok(row)
                 }
             }
 
@@ -265,8 +265,8 @@ macro_rules! tuple_impls {
             {
                 type Row = Self;
 
-                fn build(row: Self::Row) -> Self {
-                    row
+                fn build(row: Self::Row) -> deserialize::Result<Self> {
+                    Ok(row)
                 }
             }
 

diesel_cli/src/infer_schema_internals/data_structures.rs

@@ -1,6 +1,6 @@
 #[cfg(feature = "uses_information_schema")]
 use diesel::backend::Backend;
-use diesel::deserialize::{FromStaticSqlRow, Queryable};
+use diesel::deserialize::{self, FromStaticSqlRow, Queryable};
 #[cfg(feature = "sqlite")]
 use diesel::sqlite::Sqlite;
 
@@ -80,8 +80,8 @@ where
 {
     type Row = (String, String, String);
 
-    fn build(row: Self::Row) -> Self {
-        ColumnInformation::new(row.0, row.1, row.2 == "YES")
+    fn build(row: Self::Row) -> deserialize::Result<Self> {
+        Ok(ColumnInformation::new(row.0, row.1, row.2 == "YES"))
     }
 }
 
@@ -92,8 +92,8 @@ where
 {
     type Row = (i32, String, String, bool, Option<String>, bool);
 
-    fn build(row: Self::Row) -> Self {
-        ColumnInformation::new(row.1, row.2, !row.3)
+    fn build(row: Self::Row) -> deserialize::Result<Self> {
+        Ok(ColumnInformation::new(row.1, row.2, !row.3))
     }
 }
 

diesel_cli/src/infer_schema_internals/table_data.rs

@@ -1,5 +1,5 @@
 use diesel::backend::Backend;
-use diesel::deserialize::{FromStaticSqlRow, Queryable};
+use diesel::deserialize::{self, FromStaticSqlRow, Queryable};
 use std::fmt;
 use std::str::FromStr;
 
@@ -60,8 +60,8 @@ where
 {
     type Row = (String, String);
 
-    fn build((name, schema): Self::Row) -> Self {
-        TableName::new(name, schema)
+    fn build((name, schema): Self::Row) -> deserialize::Result<Self> {
+        Ok(TableName::new(name, schema))
     }
 }
 

diesel_derives/src/from_sql_row.rs

@@ -29,15 +29,15 @@ pub fn derive(mut item: syn::DeriveInput) -> Result<TokenStream, Diagnostic> {
     let (impl_generics, _, where_clause) = item.generics.split_for_impl();
 
     Ok(wrap_in_dummy_mod(quote! {
-        use diesel::deserialize::{FromSql, Queryable};
+        use diesel::deserialize::{self, FromSql, Queryable};
 
         impl #impl_generics Queryable<__ST, __DB> for #struct_ty
         #where_clause
         {
             type Row = Self;
 
-            fn build(row: Self::Row) -> Self {
-                row
+            fn build(row: Self::Row) -> deserialize::Result<Self> {
+                Ok(row)
             }
         }
     }))

diesel_derives/src/lib.rs

@@ -402,9 +402,12 @@ pub fn derive_query_id(input: TokenStream) -> TokenStream {
 /// To provide custom deserialization behavior for a field, you can use
 /// `#[diesel(deserialize_as = "SomeType")]`. If this attribute is present, Diesel
 /// will deserialize the corresponding field into `SomeType`, rather than the
-/// actual field type on your struct and then call `.into` to convert it to the
-/// actual field type. This can be used to add custom behavior for a
+/// actual field type on your struct and then call
+/// [`.try_into`](https://doc.rust-lang.org/stable/std/convert/trait.TryInto.html#tymethod.try_into)
+/// to convert it to the actual field type. This can be used to add custom behavior for a
 /// single field, or use types that are otherwise unsupported by Diesel.
+/// (Note: all types that have `Into<T>` automatically implement `TryInto<T>`,
+/// for cases where your conversion is not faillible.)
 ///
 /// # Attributes
 ///
@@ -412,8 +415,9 @@ pub fn derive_query_id(input: TokenStream) -> TokenStream {
 ///
 /// * `#[diesel(deserialize_as = "Type")]`, instead of deserializing directly
 ///   into the field type, the implementation will deserialize into `Type`.
-///   Then `Type` is converted via `.into()` into the field type. By default
-///   this derive will deserialize directly into the field type
+///   Then `Type` is converted via
+///   [`.try_into`](https://doc.rust-lang.org/stable/std/convert/trait.TryInto.html#tymethod.try_into)
+///   into the field type. By default this derive will deserialize directly into the field type
 ///
 ///
 /// # Examples
@@ -455,7 +459,7 @@ pub fn derive_query_id(input: TokenStream) -> TokenStream {
 /// #
 /// # use schema::users;
 /// # use diesel::backend::{self, Backend};
-/// # use diesel::deserialize::{Queryable, FromSql};
+/// # use diesel::deserialize::{self, Queryable, FromSql};
 /// # use diesel::sql_types::Text;
 /// #
 /// struct LowercaseString(String);
@@ -474,8 +478,8 @@ pub fn derive_query_id(input: TokenStream) -> TokenStream {
 ///
 ///     type Row = String;
 ///
-///     fn build(s: String) -> Self {
-///         LowercaseString(s.to_lowercase())
+///     fn build(s: String) -> deserialize::Result<Self> {
+///         Ok(LowercaseString(s.to_lowercase()))
 ///     }
 /// }
 ///
@@ -508,7 +512,7 @@ pub fn derive_query_id(input: TokenStream) -> TokenStream {
 /// # include!("../../diesel/src/doctest_setup.rs");
 /// #
 /// use schema::users;
-/// use diesel::deserialize::{Queryable, FromSqlRow};
+/// use diesel::deserialize::{self, Queryable, FromSqlRow};
 /// use diesel::row::Row;
 ///
 /// # /*
@@ -527,8 +531,8 @@ pub fn derive_query_id(input: TokenStream) -> TokenStream {
 /// {
 ///     type Row = (i32, String);
 ///
-///     fn build((id, name): Self::Row) -> Self {
-///         User { id, name: name.to_lowercase() }
+///     fn build((id, name): Self::Row) -> deserialize::Result<Self> {
+///         Ok(User { id, name: name.to_lowercase() })
 ///     }
 /// }
 ///

diesel_derives/src/queryable.rs

@@ -17,7 +17,7 @@ pub fn derive(item: syn::DeriveInput) -> Result<proc_macro2::TokenStream, Diagno
     let field_ty = &field_ty;
     let build_expr = model.fields().iter().enumerate().map(|(i, f)| {
         let i = syn::Index::from(i);
-        f.name.assign(parse_quote!(row.#i.into()))
+        f.name.assign(parse_quote!(row.#i.try_into()?))
     });
     let sql_type = (0..model.fields().len())
         .map(|i| {
@@ -45,18 +45,19 @@ pub fn derive(item: syn::DeriveInput) -> Result<proc_macro2::TokenStream, Diagno
     let (impl_generics, _, where_clause) = generics.split_for_impl();
 
     Ok(wrap_in_dummy_mod(quote! {
-        use diesel::deserialize::{FromStaticSqlRow, Queryable};
+        use diesel::deserialize::{self, FromStaticSqlRow, Queryable};
         use diesel::row::{Row, Field};
+        use std::convert::TryInto;
 
         impl #impl_generics Queryable<(#(#sql_type,)*), __DB> for #struct_name #ty_generics
             #where_clause
         {
             type Row = (#(#field_ty,)*);
 
-            fn build(row: Self::Row) -> Self {
-                Self {
+            fn build(row: Self::Row) -> deserialize::Result<Self> {
+                Ok(Self {
                     #(#build_expr,)*
-                }
+                })
             }
         }
     }))