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

Associations: what should they look like? #89

Closed
sgrif opened this issue Jan 12, 2016 · 16 comments
Closed

Associations: what should they look like? #89

sgrif opened this issue Jan 12, 2016 · 16 comments

Comments

@sgrif
Copy link
Member

sgrif commented Jan 12, 2016

Let's talk about associations. First what's in the public API today, what's in the private API today, and what they actually need to do. We can use those things to drive what the API should look like.

Public API today

Right now Table has inner_join and left_outer_join which can take any other table which implements JoinTo. This is limited to exactly one table today, due to some limitations that I think will be resolved by specialization. There also needs to be some boilerplate implementations of SelectableExpression for all the various types of query sources, which will go away when specialization (or rust-lang/rust#29864) lands and replaced with:

impl<Left, Right, ST, T> SelectableExpression<InnerJoinSource<Left, Right>, ST>
    for T where
    T: SelectableExpression<Left, ST>,
    Left: JoinTo<Right>,
{}

impl<Left, Right, ST, T> SelectableExpression<InnerJoinSource<Left, Right>, ST>
    for T where
    T: SelectableExpression<Right, ST>,
    Left: JoinTo<Right>,
{}

This will also basically allow us to implement the join methods on the various join sources as well, though we might need to do some tuple hackery.

Private API today

The annotations #[has_many] and #[belongs_to] are part of the internal API today. Both implement JoinTo automatically. belongs_to additionally implements BelongingToDsl<Parent> for the child. I wrote up some thoughts on where that is today at #86 (comment).

What do Associations actually need to do?

Ultimately we're either eager loading the children for a collection of parents, or we're loading the children for a single parent. I believe that BelongingTo sufficiently handles the latter, but it'll effectively be handled by the former.

One To Many

I'm pretty reasonably confident that associations should be non invasive (user doesn't know it has many posts). That means that the type of a user and all of its posts is (User, Vec<Post>). This can get tricky when dealing with multiple levels of nesting here. For example, the type of a user, all the posts they've written, and all the comments written by that user is (User, Vec<Post>, Vec<Comment>). By contrast, the type of a user, all of the posts they've written, and all of the comments left on each of those posts would be (User, Vec<(Post, Vec<Comment>)>).

I'd imagine the way to specify that you want to load the comments for the posts and not for the users is by writing users.left_outer_join(posts.left_outer_join(comments)) as opposed to users.left_outer_join(posts).left_outer_join(comments).

The case of a user and all of its posts would be written today as:

users.left_outer_joins(posts::table).load(&connection)
    .group_by(|(user, post)| user)
    .map(|(user, posts)| posts.into_iter().filter_map(|p| p).collect())
    .collect()

The case involving comments is effectively impossible today.

We do not need to have specific code to handle loading the children for a single record, as Post::belonging_to(&user) is sufficient.

It should also be noted that I don't want to restrict what you're able to work with. We should be able to get a user, and the title of all of their posts by doing users.left_outer_joins(posts::table).select((users.all_columns(), posts::title)).

One To One

The type of a one to one relationship is either (A, B) or (A, Option<B>). Technically only a belongs_to can be mandatory, but you can still end up with the first signature with an inner join. Regardless of if we're going child to parent or parent to child, the way this is written today is simply:

users.inner_join(profile)

And loading a single record is handled by Post::belonging_to(&user). I do not believe we need any additional code to handle this case, but we could potentially rename the join methods to have parity with whatever we come up with for one to many.

Composition

Associations should be composable. If we want to get all of the comments that have been written for any posts written by a user, we should be able to re-use our existing logic, without having to actually load the posts.

@sgrif
Copy link
Member Author

sgrif commented Jan 12, 2016

@mfpiccolo @mikeastock @samphippen @mcasper Would like to get some discussion going on this.

@mfpiccolo
Copy link
Contributor

Thanks for putting this together @sgrif.

One to many:
The idea here is to handle the grouping and mapping into the expected Vec

's internally correct?

Currently we have:
users.left_outer_join(posts).load(&conn).unwrap().collect() // Vec<(User, Option<Post>)>
but we want this to return: Vec<(User, Vec<Post>)>

What you have here makes sense to me as well:
users.left_outer_join(posts.left_outer_join(comments)) // Vec<(User, Vec<(Post, Vec<Comment>)>)>
users.left_outer_join(posts).left_outer_join(comments) // Vec<(User, Vec<Post>, Vec<Comment>)>

@sgrif
Copy link
Member Author

sgrif commented Jan 23, 2016

I'm moving this to 0.6, as SQLite support is more immediately actionable.

@sgrif
Copy link
Member Author

sgrif commented Aug 11, 2016

The first pass was released in 0.7, but I'm leaving this open as I think there's still more work to be done.

@beefsack
Copy link

Have been using the current iteration of associations and has been working really well in my testing.

One case I'm not sure the current implementation covers is as below; having multiple belongs_to for through different fields to the same table.

#[derive(Debug, PartialEq, Clone, Queryable, Identifiable, Associations)]
#[belongs_to(User, foreign_key="source_user_id")]
#[belongs_to(User, foreign_key="target_user_id")]
pub struct Friend {
    pub id: Uuid,
    pub created_at: NaiveDateTime,
    pub updated_at: NaiveDateTime,
    pub source_user_id: Uuid,
    pub target_user_id: Uuid,
    pub has_accepted: Option<bool>,
}

The error is:

file: 'file:///...%3CBelongsTo%20macros%3E'
severity: 'Error'
message: 'conflicting implementations of trait `diesel::associations::BelongsTo<db::models::User>` for type `db::models::Friend`:
conflicting implementation for `db::models::Friend`'
at: '37,1'
source: 'rustc'

I'm not seeing these sorts of relationships defined in the issue description so thought I would bring it up.

@sbstp
Copy link

sbstp commented Mar 19, 2018

I don't think that what @beefsack pointed out has been fixed.

Here's what I'm trying to do and the error I get

#[macro_use]
extern crate diesel;

use diesel::prelude::*;

mod schema {
    table! {
        humans {
            id -> Integer,
            name -> VarChar,
            father_id -> Integer,
            mother_id -> Integer,
        }
    }
}

use schema::*;

#[derive(PartialEq, Debug, Associations, Identifiable, Queryable)]
#[belongs_to(Human, foreign_key = "father_id")]
#[belongs_to(Human, foreign_key = "mother_id")]
struct Human {
    id: i32,
    name: String,
    father_id: i32,
    mother_id: i32,
}
error[E0119]: conflicting implementations of trait `diesel::associations::BelongsTo<Human>` for type `Human`:
  --> src/main.rs:20:28
   |
20 | #[derive(PartialEq, Debug, Associations, Identifiable, Queryable)]
   |                            ^^^^^^^^^^^^
   |                            |
   |                            first implementation here
   |                            conflicting implementation for `Human`
   |
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error: aborting due to 2 previous errors

@Raytlty
Copy link

Raytlty commented Oct 13, 2018

@sbstp I wonder if there are any solutions for this problem. THANKS.

@sbstp
Copy link

sbstp commented Oct 13, 2018

@Raytlty I think there is. Just newtype the Human struct as two different strucs like so: struct Father(pub Human), struct Mother(pub Human) and use #[belongs_to(Father, foreign_key = "father_id")]
#[belongs_to(Mother, foreign_key = "mother_id")]. Haven't tested it but it should work.

@Raytlty
Copy link

Raytlty commented Oct 15, 2018

@sbstp Thanks, I just test it and it work!

@thomasantony
Copy link

thomasantony commented Sep 20, 2019

@sbstp @Raytlty Was this ever resolved? I tried using a newtype. But this results in Associations not working because I cannot make the newtype Identifiable (which is required for Associations to work).

@sbstp
Copy link

sbstp commented Sep 20, 2019

I haven't really used diesel since making that comment, but what prevents you from implementing Identifiable on the new type?

@thomasantony
Copy link

thomasantony commented Sep 20, 2019

It tells me:

  --> src/models.rs:16:10
   |
16 | #[derive(Identifiable)]
   |          ^^^^^^^^^^^^
   |
   = help: message: All fields of tuple structs must be annotated with `#[column_name]`

Here is how I defined the structs:

#[derive(Identifiable, Queryable, Debug)]
#[table_name="accounts"]
pub struct DbAccount {
    pub id: i32,
    pub name: String,
    pub opening_balance: String,
    pub account_type_id: i32,
    pub on_budget: bool
}

#[derive(Debug)]
#[derive(Identifiable)]
#[primary_key(id)]
#[table_name="accounts"]
pub struct DbAccountFrom(pub DbAccount);

#[derive(PartialEq, Debug, Associations, Identifiable, Queryable)]
#[belongs_to(DbAccountFrom, foreign_key = "from_account_id")]
#[belongs_to(DbAccount, foreign_key="to_account_id")]
#[table_name="ledger"]
pub struct DbTransaction {
    pub id: i32,
    pub date: NaiveDate,
    pub memo: String,
    pub amount: String,
    pub from_account_id: i32,
    pub to_account_id: i32,
    pub category_id: Option<i32>,
}

I also tried making a separate newtype for the second foreign key, but the same thing happens. I am using diesel 1.4.2. Thanks!

@sbstp
Copy link

sbstp commented Sep 20, 2019

I can see why the derive does not work, it expects to see a self.id field in the struct, but that field is in self.0.id. Try manually implementing Identifiable by declaring the id method yourself. Something roughly like this:

#[derive(Debug)]
#[table_name="accounts"]
pub struct DbAccountFrom(pub DbAccount);

impl<'a> Identifiable for &'a DbAccountFrom {
  type Id = &'a u32;

  fn id(&'a self) -> &'a u32 {
    &self.0.id
  }
}

@thomasantony
Copy link

Is there any way to specify the table name as part of the impl?

@sbstp
Copy link

sbstp commented Sep 20, 2019

I'm not familiar enough with diesel to answer that, you can probably find better help here.

@thomasantony
Copy link

Thanks for the help! I will check Gitter.

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

No branches or pull requests

6 participants