Make Queryable faillible

One approach to fixing diesel-rs#2523
Ten0 · Dec 21, 2020 · f9751b0 · f9751b0
1 parent 06494e8
commit f9751b0
Show file tree

Hide file tree

Showing 9 changed files with 34 additions and 33 deletions.
diff --git a/diesel/src/deserialize.rs b/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)
     }
 }
 

diff --git a/diesel/src/pg/types/ranges.rs b/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)
     }
 }
 

diff --git a/diesel/src/pg/types/record.rs b/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)
             }
         }
 

diff --git a/diesel/src/type_impls/option.rs b/diesel/src/type_impls/option.rs
@@ -105,8 +105,8 @@ where
 {
     type Row = Self;
 
-    fn build(row: Self::Row) -> Self {
-        row
+    fn build(row: Self::Row) -> deserialize::Result<Self> {
+        Ok(row)
     }
 }
 

diff --git a/diesel/src/type_impls/primitives.rs b/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)
     }
 }
 

diff --git a/diesel/src/type_impls/tuples.rs b/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)
                 }
             }
 

diff --git a/diesel_derives/src/from_sql_row.rs b/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)
             }
         }
     }))

diff --git a/diesel_derives/src/lib.rs b/diesel_derives/src/lib.rs
@@ -508,7 +508,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 +527,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() })
 ///     }
 /// }
 ///

diff --git a/diesel_derives/src/queryable.rs b/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,)*
-                }
+                })
             }
         }
     }))