Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

ManyToMany Relationship #328

Closed
b1zzu opened this issue Sep 9, 2019 · 21 comments
Closed

ManyToMany Relationship #328

b1zzu opened this issue Sep 9, 2019 · 21 comments

Comments

@b1zzu
Copy link
Contributor

b1zzu commented Sep 9, 2019

Having a ManyToMany relationship could spare some boiler template work and also make the database more performant.

For example, I have this schema:

type User {
  id: ID!
  name: String!
  activities: [Activity]! @ManyToMany(field: "partecipants")
}

type Activity {
  id: ID!
  name: String!
  description: String
}

where a User can participate to has many Activities and an Activity can have many participants (Users).

This will be the expected graphql schema:

type User {
  id: ID!
  name: String!
  activities: [Activity]!
}

type Activity {
  id: ID!
  name: String!
  description: String
  partecipants: [User]!
}

From the queries point of view, we can implement an optimized resolver for activities and participants, but they should be straight forward.

Instant for the mutations I have a couple of ideas:

  1. An add/remove participant or activity
type Mutation {
  addPartecipantToActivity(activityId: ID!, userId: ID!): Activity!
  removePartecipantFromActivity(activityId: ID!, userId: ID!) Activity!
  addActivityToUser(userId: ID!, activityId: ID!): User!
  removeActivityFromUser(userId: ID!, activityId: ID!): User!
}
  1. Bulk update participants or activities
input ActivityInput {
  # other fields
  partecipants: [ID]!
}

input UserInput {
  # ...
  activities: [ID]!
}

# and the standard update and crate mutations

And finally in the database should be created a table called activieties_to_partecipants with the userId and activityId foreign keys, and usually, the id would be the combination of the two foreign keys, but it's not always like that.

It's also important to keep in mind that it could be useful to personalize the DB table name, the foreign keys names, and the mutations names (if using the first option)

@wtrocki
Copy link
Contributor

wtrocki commented Sep 9, 2019

Thank you so much @b1zzu.
I think we had experimented with this.

Ideally we need to have coverage for two cases:

  • ManyToMany with explicit GraphQL type that will map to the joined table but it will be ignored for CRUD generation
  • ManyToMany out of the box defined on each node (Mapping to the same table)

@b1zzu
Copy link
Contributor Author

b1zzu commented Sep 9, 2019

@wtrocki both solutions look good, but we need to ensure that performance speaking one is not worse than the other and with this a mean that we should perform one single query in order to fetch the relation.

For example, I have temporarily solved the many to many relation in this way, using an additional type as a join table

type User {
  id: ID!
  name: String!
  activities: [Partecipant]!
}

type Participant {
  id: ID!
}

type Activity {
  id: ID!
  name: String!
  description: String
  participants: [Participant]!
}

but this has the big disadvantage that if I want to query (graphql) one Activity with all the participant's name I will perform a db query for each participant user.

Apart from this, I would prefer a ManyToMany relation out of the box, but it could be also interesting to have both solutions, especially in the case a user would like to have additional fields in the on the join relation.

@wtrocki
Copy link
Contributor

wtrocki commented Sep 12, 2019

@ankitjena It looks like we have capabilities for Many2Many but they are not documented.
I have tried @manytomany directive and it seems to be creating database resources properly:
Screenshot 2019-09-12 at 11 26 12

However there seems to be a problem with how to model schema in order to support it.
For example this case is not valid and it will not allow us to query data:
Screenshot 2019-09-12 at 11 26 23

Resolvers in this case still using FK instead of relationship table.

Is missing resolver support a reason for missing ManyToMany documentation?
I totally forgot what was the initial assumption here and generally, it feels that
ManyToMany is not properly handled now. Maybe we can remove it for the moment and get proper thoughts on how to do it properly.

@ankitjena
Copy link
Collaborator

ankitjena commented Sep 12, 2019

However there seems to be a problem with how to model schema in order to support it.

This was the reason why we removed it from documentation. We have not handled resolver implementation for this.

ManyToMany with explicit GraphQL type that will map to the joined table but it will be ignored for CRUD generation.

I think this will be good idea since we don't know what will be the user's usecases, and manytomany is not frequent.

Suppose a case we need a ManyToMany relationship between User and Note, when implementing a custom resolver like findNotesByUser we can query from the table the ids and select from the respective table accordingly. The implementation will depend on the user's need and we can skip it for CRUD generation.

@wtrocki @b1zzu WDYT?

@wtrocki
Copy link
Contributor

wtrocki commented Sep 12, 2019

I think we might go with:

  • Removing the current implementation first to not confuse people as directive is still there
  • Have investigation how to use relationship table.

Proposed suggestion seems to be quick wins but ManyToMany is so important feature that we might just do it properly.

@wtrocki
Copy link
Contributor

wtrocki commented Sep 22, 2019

https://github.com/prisma/prisma2/blob/master/docs/relations.md#mn good read for possible approach

@b1zzu
Copy link
Contributor Author

b1zzu commented Sep 23, 2019

The ManyToMany relation in Django is one of the best I've ever seen (https://docs.djangoproject.com/en/2.2/topics/db/examples/many_to_many/#) maybe we can take some ideas from it

@wtrocki wtrocki self-assigned this Nov 6, 2019
@wtrocki wtrocki removed their assignment Nov 18, 2019
@wtrocki wtrocki pinned this issue Dec 16, 2019
@wtrocki wtrocki unpinned this issue Dec 16, 2019
@craicoverflow
Copy link

craicoverflow commented Dec 16, 2019

With the new migration engine integrated bringing ManyToMany table generation this moves one step closer.

The @db.manyToMany annotation creates a many-to-many table.

Next steps: queries, mutations, subscriptions.

[Requires further investigation] I think we would get away without any changes to the generated schema and that the majority of changes would happen at resolver generation phase.

@craicoverflow
Copy link

For many-to-many support at schema and resolver level, we have been considering letting users define two 1:M relationships and creating a many-to-many type in their model.

AWS Amplify does something similar:

aws-amplify/amplify-cli#91 (comment)
https://forums.aws.amazon.com/thread.jspa?messageID=865055

@wtrocki
Copy link
Contributor

wtrocki commented Jan 30, 2020

Awesome! Then we should hide many2many on db as it will cause confusion

@wtrocki
Copy link
Contributor

wtrocki commented Feb 12, 2020

We need documentation to cover this case and some example on top of the example apps to see if that approach will work

@wtrocki wtrocki added the good first issue Good for newcomers label Feb 12, 2020
@craicoverflow
Copy link

Awesome! Then we should hide many2many on db as it will cause confusion

Yep we should completely disable it.

@craicoverflow
Copy link

This can be closed since all that is required is documentation, which will come when we document relationships.

@wtrocki
Copy link
Contributor

wtrocki commented Apr 26, 2020

Still not documented. User reported issue. Relationship documentation is hidden in wrong chapter https://graphback.dev/docs/db/dbmigrations#foreign-keys

@wtrocki wtrocki reopened this Apr 26, 2020
@craicoverflow
Copy link

Relationship documentation is also in data model section:
https://graphback.dev/docs/intro/datamodel#relationships

The bug isn't an issue with the documentation, but we should specify how to create a M:M

@wtrocki
Copy link
Contributor

wtrocki commented Apr 27, 2020

Data model on is shorter - we are missing:

  • info about bidirectional relationships generated
  • info about ManyToOne ( available as option)
  • info about ManyToMany ( With helper table )

@craicoverflow
Copy link

Adding ManyToMany now.

  1. Information about generating bidirectional fields is there in DataModel:
This creates a one-to-many relationship between Note.comments and Comment.note. If Comment.note does not exist Graphback will generate it for you, otherwise you can define it yourself.
  1. We haven't documented ManyToOne because manually specifying ManyToOne will not generate the metadata or OneToMany field.
    We decided not to implement this because it is less confusing and easier to add the OneToMany side.

  2. Adding ManyToMany info now.

@wtrocki
Copy link
Contributor

wtrocki commented Apr 27, 2020

We haven't documented ManyToOne because manually specifying ManyToOne will not generate the metadata or OneToMany field.

Oh.. I did not know about it. Good to add this to docs as well.

@wtrocki
Copy link
Contributor

wtrocki commented May 6, 2020

Docs landed but I still think we need to have better support for this kind of relationship

@machi1990
Copy link
Contributor

Docs landed but I still think we need to have better support for this kind of relationship

Agreed. I am trying out the suggested solutions in the doc in OVP, will try to gather some ideas.

@craicoverflow
Copy link

Closing as duplicate of #2023

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

No branches or pull requests

5 participants