Repository: diesel-rs/diesel (language: rust)

Issue:
Selectable derive does not work with generic embed sub-fields
<!--
If you want to report a bug, we added some points below which help us track down the problem faster.
-->

## Setup

### Versions

- **Diesel:** Latest master (b2c58897c39c519a946314bd5b63765d3af56204)


### What are you trying to accomplish?
```rust
#[derive(Queryable, Selectable)]
#[table_name = "schema::xxx"]
pub struct WithField<T> {
	pub field: i32,
	#[diesel(embed)]
	pub inner: T,
}
```

### What is the expected output?
```rust 
impl<T, __DB> Selectable<__DB> for WithField<T>
where T: Selectable<__DB> {}
```

### What is the actual output?
```rust 
impl<T, __DB> Selectable<__DB> for WithField<T> {}
```
-> fails to compile with "the trait bound `T: diesel::Selectable<__DB>` is not satisfied".

### Solving

Looks like if a field has `embed`, the `Selectable` derive should add bounds `FieldType: diesel::Selectable<__DB>`, regardless of whether the type is generic or not.

This would also allow the `Selectable` derive to interact as expected with manual implementations of `Selectable` on a single backend (non-generic).


Discussion / hints:
I'm not sure if there is an easy way to fix this issue. We would need to do some simplified form of type resolution there to understand that the type `T` is a generic parameter to restrict it to `T: Selectable`.  We cannot add this bound for any generic parameter, as this would potentially add the bound to non `#[diesel(embedded)]` fields as well, which may result in unwanted behaviour there.

To workaround this issue, you can add this bound directly to your struct definition. This should also force the bound in any generated code.
> We cannot add this bound for any generic parameter, as this would potentially add the bound to non `#[diesel(embedded)]` fields as well, which may result in unwanted behaviour there.

My suggestion was to add the bound not on the generic types, but on the **field types that are `embed`**.

e.g., here:
```rust
#[derive(Selectable)]
struct S<T> {
    #[diesel(embed)]
    a: SomeStruct,
    #[diesel(embed)]
    b: T,
    some_field: i32,
}
```
the where clause would be `where SomeStruct: Selectable<__DB>, T: Selectable<__DB>`

> This would also allow the Selectable derive to interact as expected with manual implementations of Selectable on a single backend (non-generic).

e.g.
```rust
struct S {
   ...
}
impl Selectable<Pg> for S { ... }
#[derive(Selectable)]
struct S2 {
    #[diesel(embed)]
    f: S,
}
```

> To workaround this issue, you can add this bound directly to your struct definition. This should also force the bound in any generated code.
 
I've considered this but the bound is relative to a `__DB` type that doesn't exist at that time. For now I've just written the impl manually.
> My suggestion was to add the bound not on the generic types, but on the field types that are embed.
> 
> e.g., here:
> ```
> #[derive(Selectable)]
> struct S<T> {
>    #[diesel(embed)]
>    a: SomeStruct,
>    #[diesel(embed)]
>    b: T,
>    some_field: i32,
> }
>```
> the where clause would be `where SomeStruct: Selectable<__DB>, T: Selectable<__DB>> `

That's a great idea and that should be really simple to add. Basically just add a loop over fields here and add the corresponding bounds to the where clause:
https://github.com/diesel-rs/diesel/blob/b2c58897c39c519a946314bd5b63765d3af56204/diesel_derives/src/selectable.rs#L12-L16

```rust


for field in model.fields() {
    if field.has_flag("embed") {
        let where_clause = generics.where_clause.get_or_insert(parse_quote!(where));
        let embed_ty = &field.ty;

        where_clause
                .predicates
                .push(parse_quote!(#embed_ty: Selectable<__DB>));
    }
}
```

Feel free to open a PR with this + a test case showing that it works as expected.


Fix patch:
diff --git a/diesel_derives/src/selectable.rs b/diesel_derives/src/selectable.rs
index 1f6b2446c77d..4417ab88c7de 100644
--- a/diesel_derives/src/selectable.rs
+++ b/diesel_derives/src/selectable.rs
@@ -9,10 +9,19 @@ pub fn derive(item: syn::DeriveInput) -> Result<proc_macro2::TokenStream, Diagno
     let model = Model::from_item(&item)?;
 
     let (_, ty_generics, _) = item.generics.split_for_impl();
+
     let mut generics = item.generics.clone();
     generics
         .params
         .push(parse_quote!(__DB: diesel::backend::Backend));
+    for embed_field in model.fields().iter().filter(|f| f.has_flag("embed")) {
+        let embed_ty = &embed_field.ty;
+        generics
+            .where_clause
+            .get_or_insert_with(|| parse_quote!(where))
+            .predicates
+            .push(parse_quote!(#embed_ty: Selectable<__DB>));
+    }
     let (impl_generics, _, where_clause) = generics.split_for_impl();
 
     let struct_name = &item.ident;


Test patch:
diff --git a/diesel_derives/tests/selectable.rs b/diesel_derives/tests/selectable.rs
index 81d959a9efe9..45729b5e119f 100644
--- a/diesel_derives/tests/selectable.rs
+++ b/diesel_derives/tests/selectable.rs
@@ -44,7 +44,7 @@ fn tuple_struct() {
 fn embedded_struct() {
     #[derive(Debug, Clone, Copy, PartialEq, Eq, Queryable, Selectable)]
     #[table_name = "my_structs"]
-    struct A {
+    struct A<B> {
         foo: i32,
         #[diesel(embed)]
         b: B,
@@ -52,12 +52,14 @@ fn embedded_struct() {
 
     #[derive(Debug, Clone, Copy, PartialEq, Eq, Queryable, Selectable)]
     #[table_name = "my_structs"]
-    struct B {
+    struct C {
         bar: i32,
     }
 
     let conn = &mut connection();
-    let data = my_structs::table.select(A::as_select()).get_result(conn);
+    let data = my_structs::table
+        .select(A::<C>::as_select())
+        .get_result(conn);
     assert!(data.is_err());
 }