Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Note join extract struct not supported #2627

Closed
wants to merge 1 commit into from

Conversation

pickfire
Copy link
Contributor

@pickfire pickfire commented Jan 20, 2021

As I saw (User, (Post, Comment)) in the docs, I thought using join to extract the original struct out without using select works, I spent some time trying it out and I believe it does not work. It would be better to tell others that select is still needed without letting them try it since it will fail during compilation time.

Or maybe we can explain this is due to ambiguity?

@weiznich
Copy link
Member

As we do exactly that what you describe as impossible as part of our test suite I would rather not like to have this sentence in our documentation (as this is actually possible and supported.)

@pickfire
Copy link
Contributor Author

Oh, weird maybe because I was doing multiple joins that it make it not working? I guess I will close it and reinvestigate.

@pickfire pickfire closed this Jan 21, 2021
@pickfire pickfire deleted the patch-1 branch January 21, 2021 04:29
@pickfire
Copy link
Contributor Author

Ah, I think it is because the struct does not have all the fields in the original schema.

table! {                                
    comments (id) {                     
        id -> Integer,                  
        body -> Text,                   
        post_id -> Integer,             
        user_id -> Integer,             
    }                                   
}                                       
                                        
table! {                                
    posts (id) {                        
        id -> Integer,                  
        title -> Text,                  
        body -> Nullable<Text>,         
        user_id -> Integer,             
    }                                   
}                 

allow_tables_to_appear_in_same_query!(     
    comments,                              
    posts,                                 
    users,
);

joinable!(comments -> users (user_id));                       

#[derive(Queryable, Debug)]
pub struct Post {
    pub id: i32,
    pub title: String,
    pub body: Option<String>,
    pub user_id: i32,
}
                                      
#[derive(Queryable, Debug)]           
pub struct Comment {                  
    pub id: i32,                      
    pub body: String,                 
}                                     

fn main() {
    ...
    let query = comments.inner_join(posts);
    let data: Vec<(Comment, Post)> = query.load(&conn).unwrap();
}

The error.

error[E0277]: the trait bound `(i32, String): Queryable<(diesel::sql_types::Integer, diesel::sql_types::Text, diesel::sql_types::Integer, diesel::sql_types::Integer), Sqlite>` is not satisfied
  --> src/main.rs:24:44
   |
24 |     let data: Vec<(Comment, Post)> = query.load(&conn).unwrap();
   |                                            ^^^^ the trait `Queryable<(diesel::sql_types::Integer, diesel::sql_types::Text, diesel::sql_types::Integer, diesel::sql_types::Integer), Sqlite>` is not implemented for `(i32, String)`
   |
   = help: the following implementations were found:
             <(A, B) as Queryable<(SA, SB), __DB>>
   = note: required because of the requirements on the impl of `Queryable<(diesel::sql_types::Integer, diesel::sql_types::Text, diesel::sql_types::Integer, diesel::sql_types::Integer), Sqlite>` for `Comment`
   = note: required because of the requirements on the impl of `Queryable<((diesel::sql_types::Integer, diesel::sql_types::Text, diesel::sql_types::Integer, diesel::sql_types::Integer), (diesel::sql_types::Integer, diesel::sql_types::Text, diesel::sql_types::Nullable<diesel::sql_types::Text>, diesel::sql_types::Integer)), Sqlite>` for `(Comment, Post)`
   = note: required because of the requirements on the impl of `LoadQuery<SqliteConnection, (Comment, Post)>` for `SelectStatement<JoinOn<diesel::query_source::joins::Join<hello::schema::comments::table, hello::schema::posts::table, Inner>, diesel::expression::operators::Eq<diesel::expression::nullable::Nullable<post_id>, diesel::expression::nullable::Nullable<hello::schema::posts::id>>>>`

error: aborting due to previous error; 3 warnings emitted

For more information about this error, try `rustc --explain E0277`.
error: could not compile `hello`

To learn more, run the command again with --verbose.

I believe it's because the Comment is not taking in all the data in the schema. Rather than having id, body, post_id, user_id I only have id, body for Comment. Maybe perhaps that is the case that is not supported?

If you want I can provide a reproducible case.

@weiznich
Copy link
Member

@pickfire I'm not sure why you find that surprising. That's the normal behaviour of #[derive(Queryable)]. It applies to all queries, not only to joins. If you remove the join and only try to load Comment in your example it will fail with nearly the same error message.

@pickfire
Copy link
Contributor Author

pickfire commented Jan 21, 2021

Oh, I wasn't aware of this. Quite a surprise to me, I tried it without join it also didn't work.

I did a quite search in the docs https://docs.rs/diesel/1.4.5/diesel/deserialize/trait.Queryable.html

When this trait is derived, it will assume that the order of fields on your struct match the order of the fields in the query. This means that field order is significant if you are using #[derive(Queryable)]. Field name has no effect.

It only mentions that field order is important.

And this

Types which implement Queryable represent the result of a SQL query. This does not necessarily mean they represent a single database table.

Now I am confused. It says it does not necessary mean they represent a single database table.

Is there a way to join but extract only part of it from the table? IIRC we can query part of the data right?

@weiznich
Copy link
Member

@pickfire First of all: This is not really the right place for this discussion. Please use the gitter channel for such questions.

It only mentions that field order is important.

If it's a 1:1 mapping it implies that it cannot skip fields in my opinion.

Is there a way to join but extract only part of it from the table? IIRC we can query part of the data right?

Yes there is. See the documentation on QueryDsl::select for how to skip columns. I mean everything #[derive(Queryable)] does is taking a tuple of n values and mapping those values to a struct with the same number of fields. Not more not less, so there is nothing table specific there.

@pickfire
Copy link
Contributor Author

pickfire commented Jan 22, 2021

Sorry, I have no more questions.

If it's a 1:1 mapping it implies that it cannot skip fields in my opinion.

Queryable documentation does not make it clear that it is a one-to-one mapping.

@weiznich
Copy link
Member

Queryable documentation does not make it clear that it is a one-to-one mapping.

Again this is the behaviour of #[derive(Queryable)] not of the trait Queryable The corresponding documentation mentions the following things (highlighted by me):

1.4.x:

When this trait is derived, it will assume that the order of fields on your struct match the order of the fields in the query. This means that field order is significant if you are using #[derive(Queryable)]. Field name has no effect.

It clearly says that the fields must match in order, which implies that there must be the same number of fields. If you have suggestions how to improve this formulation while not exclude use cases that have a custom setup somewhere feel free to submit a PR.

The documentation for the upcoming 2.0 release has even an example which code is generated.

@Mingun
Copy link
Contributor

Mingun commented Jan 22, 2021

which implies that there must be the same number of fields

IMHO, no, doesn't imply

@weiznich
Copy link
Member

@Mingun If you believe that feel free to submit a PR to improve that wording. Otherwise: I do not really like people coming up again and again criticizing (fundamental) things without providing any justification and without contributing otherwise to the project. Either start contributing something or add something actionable to your comments or stop making them as they are currently not helpful for any of the contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants