Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

[Preview 14] Migrate error - ambiguous self relation detected #290

Closed
nhuesmann opened this issue Oct 14, 2019 · 15 comments
Closed

[Preview 14] Migrate error - ambiguous self relation detected #290

nhuesmann opened this issue Oct 14, 2019 · 15 comments
Assignees
Labels
bug/2-confirmed We have confirmed that this is a bug. kind/bug A reported bug.

Comments

@nhuesmann
Copy link

Related to prisma/prisma#488 and prisma/prisma#671

Prisma2 version:

prisma2@2.0.0-preview014, binary version: 188a379c9b8c6650e5812f9bdb7407d7b197692f

Schema:

model Game {
  id           String   @id @default(cuid())
  createdAt    DateTime @default(now())
  updatedAt    DateTime @updatedAt
  title        String
  handle       String
  description  String
  expansions   Game[]   @relation("Expansion")
  expands      Game[]   @relation("Expansion")
  relatedGames Game[]  👈 this is the problem
}

Notes:

When I try to perform prisma2 lift up I get "ambiguous self relation detected" due to the relatedGames field. If I try to add the @relation directive, I get an error when I try to perform prisma lift save in which the schema can't be validated bc Named relations require an opposite field. In my model, the expansions/expands fields are different, because a Game can either have expansions, or it itself can be an expansion for several other games (meaning it will have several Games in its expands field). The relatedGames field doesn't have any concept of different roles in a relationship like expansions and expands, it's simply a way to model that games can be "related" to each other, i.e. for similar game recommendations. What should I do to resolve this?

@divyenduz
Copy link

@pantharshit00 Please triage this and if this is a bug, apply the candidate label to it 🙏

@pantharshit00
Copy link
Contributor

I can confirm this bug. Strangely enough prisma2 dev works but prisma2 lift save throws the reported bug.

Also, adding the directive also throws an error:
image

Reproduction: https://github.com/harshit-test-org/prisma-prisma2-752

@mavilein
Copy link
Member

mavilein commented Oct 15, 2019

@nhuesmann:
The solution is to to provide an opposite field:

model Game {
  id           String   @id @default(cuid())
  createdAt    DateTime @default(now())
  updatedAt    DateTime @updatedAt
  title        String
  handle       String
  description  String
  expansions   Game[]   @relation("Expansion")
  expands      Game[]   @relation("Expansion")
  relatedGames Game[]  @relation("RelatedGames")
  relatedGamesBack Game[]  @relation("RelatedGames")
}

@abdusamadtv
Copy link

@nhuesmann:
The solution is to to provide an opposite field:

model Game {
  id           String   @id @default(cuid())
  createdAt    DateTime @default(now())
  updatedAt    DateTime @updatedAt
  title        String
  handle       String
  description  String
  expansions   Game[]   @relation("Expansion")
  expands      Game[]   @relation("Expansion")
  relatedGames Game[]  @relation("RelatedGames")
  relatedGamesBack Game[]  @relation("RelatedGames")
}

what does it mean Back relation? is it required in the future?

@nhuesmann
Copy link
Author

nhuesmann commented Oct 18, 2019

NOTE: I accidentally tapped "close and comment" rather than "comment" as I'm writing this on my phone. Ignore those GitHub status updates please.

@mavilein Interesting...

Say I have a Game with id 1. If I add Games with ids 2, 3, and 4 to Game 1's relatedGames field, will Games 2, 3, and 4 have Game 1 in their relatedGamesBack field? If so, that is not how Prisma 1 works, and that is not how previous Prisma2 previews have worked. I really would like to maintain the functionality of:

  • Games added to Game 1's relatedGames field also have Game 1 added to their relatedGames fields.
    I see other developers with the same requirements. Issue Ambiguous self relation detected prisma#671 is the same thing—a User model with a friends: User[] field. It would make sense that all User records connected via the friends field would have references to their back connection in that very same field. There's no sense of relational direction/hierarchy in a friends field, just like my relatedGames field.

If I'm wrong, and functionality I mentioned I needed is still present, please let me know. Either way, I'd recommend making that clearer in the docs if possible. As I said I've seen other devs in other issues express the expectation that they'd be able to have a single field that references the model it's a part of, so I don't think I'm alone in the confusion.

@divyenduz @pantharshit00 feel free to hop in if you have a good answer to this question as well.

@nhuesmann nhuesmann reopened this Oct 18, 2019
@mavilein
Copy link
Member

@nhuesmann : Your observation is correct that we introduced a behavioral change at some point. But i am fairly certain that we introduced this change already with Prisma 1. So i am a bit confused that you are saying that even some Prisma 2 previews had the old behavior.
However it was a conscious decision to change it. There were 2 reasons for this:

  • It was also not possible to opt out of this behavior at all from a semantic perspective. (Some people want relatedGames and relatedGamesBack to be separate)
  • This feature incurred a performance cost for every query query no matter you wanted that behavior or not.

Before the change this kind of relation was a special thing and behaved differently from all other relations in our system.

What i would recommend here is to explicitly handle this usecase in your application layer. If you want this behavior you should select the fields relatedGames and relatedGamesBack in your query and union those arrays.

@nhuesmann
Copy link
Author

@mavilein Thanks for your reply. Regarding when this change occurred, I can't say exactly when it was, however I will say that I'm running preview 10 and still have the single relatedGames field and no issues. When I upgrade to any later preview version, I encounter the ambiguous self relation issue.

I understand the reasoning behind changing it. Have to admit I'm a little bummed about it, but I get that you can't satisfy every user desire and every use case, and that you're optimizing for performance. Thanks again for the clarification and recommendations, I appreciate it.

@nhuesmann
Copy link
Author

@mavilein I just thought of another question / potential issue with this approach. Say I have a User model that looks like this:

model User {
  id        ID
  name      String
  friends   User[]
  ...
}

Based on your last recommendation, the User model also needs a friendsBack field to serve as the opposite field to friends, and queries would just need to fetch both the friends field and friendsBack field then union the results.

Let's assume I have some UI that lets a User view their friends list (alphabetized), and that I want to only fetch the first 20 results then lazy load more as the user approaches the end of the list. I would ideally run a query that fetches the logged in user's friends field and passes limit and offset variables (and sortBy to sort alphabetically). However, if I have to fetch friends and friendsBack then join the arrays, doesn't this use case break? What if friends contains users with names starting with A - M, and friendsBack contains users with names starting with N - Z? If I'm trying to present an alpha-sorted list of friends, the first batch of results will have names starting with A followed by names starting with N—then when a user scrolls far enough to trigger the lazy load data fetching, the offset will be passed, resulting in users with names potentially beginning with B being added (after we've already displayed the N's).

Please let me know if this makes sense, and if you understand why I fear this may actually end up being a problem. I don't want to have to write my own subpar implementations of awesome Prisma features like sortBy, limit and offset. I believe that having a self-relation that is a single field without a separate "back" field is a pretty common use case (i.e. a "friends" field), and may even be necessary for a scenario like this. Please let me know if you have any suggestions for how to handle this scenario given the current planned architecture of Prisma2. Thank you in advance!

@tomhoule tomhoule removed their assignment Nov 7, 2019
@mavilein
Copy link
Member

mavilein commented Nov 8, 2019

@nhuesmann : Your concerns are right about this one. This still does not warrant the introduction of such a feature in my opinion since it occurs a performance cost for every relation.
I understand that the solution of joining the arrays of both relation fields feels cumbersome. My advice in this case is to lift this relation into a proper model in your application. I would introduce a model called Friendship that contains two relation fields pointing to the befriended users. This would enable you write a query that gets you all friendships for a user. This would relieve you from joining those arrays manually.

Example schema

model User {
  id Int @id
  friendships  Friendship[]
  ...
}

model Friendship {
  id Int @id
  user1 User
  user2 User
}

Example Photon call: (This is best effort code. Maybe this does not work exactly like that. Should convery the idea though)

const friendships: Friendship[] = await photon.friendships.findMany({
  where: { 
      OR: [
          { user1: { id: 'abc' }},
          { user2: { id: 'abc' }},
      ]
  },
});

@jondewoo
Copy link

jondewoo commented Nov 23, 2019

This approach makes the prisma2 CLI crash (#976).

@divyenduz divyenduz pinned this issue Jan 9, 2020
@divyenduz divyenduz unpinned this issue Jan 9, 2020
@divyenduz divyenduz transferred this issue from prisma/prisma Jan 9, 2020
@divyenduz divyenduz added bug/1-repro-available A reproduction exists and needs to be confirmed. kind/bug A reported bug. bug/2-confirmed We have confirmed that this is a bug. and removed bug/1-repro-available A reproduction exists and needs to be confirmed. labels Jan 9, 2020
@nhuesmann
Copy link
Author

nhuesmann commented Jan 13, 2020

@nhuesmann : Your concerns are right about this one. This still does not warrant the introduction of such a feature in my opinion since it occurs a performance cost for every relation.
I understand that the solution of joining the arrays of both relation fields feels cumbersome. My advice in this case is to lift this relation into a proper model in your application. I would introduce a model called Friendship that contains two relation fields pointing to the befriended users. This would enable you write a query that gets you all friendships for a user. This would relieve you from joining those arrays manually.

Example schema

model User {
  id Int @id
  friendships  Friendship[]
  ...
}

model Friendship {
  id Int @id
  user1 User
  user2 User
}

Example Photon call: (This is best effort code. Maybe this does not work exactly like that. Should convery the idea though)

const friendships: Friendship[] = await photon.friendships.findMany({
  where: { 
      OR: [
          { user1: { id: 'abc' }},
          { user2: { id: 'abc' }},
      ]
  },
});

@mavilein I just ran into this issue while trying to implement a Friendship model. My model is very similar to the example you provided in the quoted comment above. My simplified schema:

model User {
  id          Int          @id @default(autoincrement())
  friendships Friendship[]
  ...
}

enum FriendshipStatus {
  PENDING
  ACCEPTED
  DECLINED
  BLOCKED
}

model Friendship {
  id             Int              @id @default(autoincrement())
  status         FriendshipStatus @default(PENDING)
  user1          User
  user2          User
  lastActionUser User
}

Schema parsing fails with the following message:

Error validating model "Friendship": Ambiguous relation detected. The fields user1 and user2 in model Friendship both refer to User. Please provide different relation names for them by adding `@relation().

It doesn't make sense to differentiate user1 and user2 with relation names since they both point to the same field (friendships) on the User model and contain references to 2 different User records. As discussed previously, this feels like a very common use case. I can get the model "working" by just storing strings in user1, user2, and lastActionUser, however if I wanted to retrieve a user's friends, I'd have to make a similar query to what you listed above to find the friendships, then I'd have to make a separate query for the user data for each friendship since only the plain string ids are stored.

I can even do without having the friendship field on the User model, I can build that resolver functionality easily. I really just need the ability for a Friendship to have 3 fields that each point to a User record.

I'd love to hear your/anyone else from Prisma team's thoughts, thanks!

@nhuesmann
Copy link
Author

@divyenduz @mavilein I just wanted to follow up after a few weeks to see if this was going to be in any upcoming sprints. This is an important feature for me, as I need to model a friendship and currently can't (unless I figure out some undesirable workarounds, but they'd ultimately be subpar solutions).

@nhuesmann
Copy link
Author

@divyenduz @mavilein Hi guys, just following up again after a few more weeks to see if there are any plans to fix this bug. I have a feature that depends heavily on this functionality. Thanks!

@mavilein
Copy link
Member

mavilein commented Feb 18, 2020

@nhuesmann : There are no plans to change this behavior. My suggestion was more pseudo a spontaneous idea and i just realized that it won't work like that. The following schema should work:

model User {
  id Int @id
  friendships1  Friendship[] @relation("Friendship1")
  friendships2  Friendship[] @relation("Friendship2")
}

model Friendship {
  id Int @id
  user1 User @relation("Friendship1")
  user2 User @relation("Friendship2")
}

This schema should enable the query i sketched in an earlier comment.

@nhuesmann
Copy link
Author

@mavilein Thanks for the reply. I'll admit I'm a little bummed about having to set up these arbitrary back relation fields, but I understand there's a perf reason behind Prisma's decision to do this. I'll just hope that down the road there might be a slightly cleaner solve for this, but in the mean time I think this will take care of what I need. Thanks again.

@Jolg42 Jolg42 changed the title [Preview 14] Lift error - ambiguous self relation detected [Preview 14] Migrate error - ambiguous self relation detected May 26, 2020
@Jolg42 Jolg42 closed this as completed May 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug/2-confirmed We have confirmed that this is a bug. kind/bug A reported bug.
Projects
None yet
Development

No branches or pull requests

8 participants