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
fix: regression on 1-1 nested connect in create #3514
Conversation
Identifier::Simple if on_child.is_to_one_opt() && on_parent.is_to_one_opt() => { | ||
vec![RelationReference::SimpleParentId(on_child), RelationReference::NoRef] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sure we generate optional 1-1 relation inlined on both sides (child and parent).
It's a bit hard to follow what this does without the full context. In a nutshell, RelationReference::NoRef
means some of the generated datamodels will not be inlined on the child side. Therefore, the logic for the parent side will inline the relation when it's not already inlined in the child.
This means we can expect generated datamodels of these (rough) shapes:
(inlined child-side)
model Parent {
id Int @id
childOpt Child?
}
model Child {
id Int @id
parentId Int? @unique
parentOpt? @relation(fields: [parentId], references: [id])
}
(inlined parent-side)
model Parent {
id Int @id
childId Int? @unique
childOpt Child? @relation(fields: [childId], references: [id])
}
model Child {
id Int @id
parentOpt?
}
if parent_is_create { | ||
handle_one_to_one_parent_create(graph, parent_node, parent_relation_field, filter, child_model) | ||
} else { | ||
handle_one_to_one_parent_update(graph, parent_node, parent_relation_field, filter, child_model) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff of the split is not super easy to read. This code mainly removed most of the is_parent_create
checks and inlined everything in separate functions.
It made it easier to only do the idempotency check on nested connect inside update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's easier to read than before, so 👍🏾
query-engine/core/src/query_graph_builder/write/nested/disconnect_nested.rs
Outdated
Show resolved
Hide resolved
// If the relation is inlined on the parent and a filter is applied on the child, we need to join to apply this filter | ||
let filter = if !filter.is_empty() { | ||
parent_relation_field.to_one_related(filter) | ||
} else { | ||
filter | ||
}; | ||
|
||
(parent_node, parent_model, extractor_model_id, null_record_id) | ||
(parent_node, parent_model, extractor_model_id, null_record_id, filter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the fix for the nested disconnect bug. When the relation is inlined on the parent and a filter is applied on the child, we need to convert the filter to a relation filter so that a join is rendered. eg:
model Parent {
id Int @id
childId Int? @unique
childOpt Child? @relation(fields: [childId], references: [id])
}
model Child {
id Int @id
non_unique Int
parentOpt?
}
await prisma.parent.update({
where: { id: 1 },
data: {
childOpt: { disconnect: { non_unique: 1 } }
}
})
This should roughly render:
UPDATE Parent SET childId = NULL
LEFT JOIN Child c ON (c.id = Parent.childId)
WHERE Child.non_unique = 1
Note: We do not use joins for relational filters. This some pseudo code.
9002093
to
be5934c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look correct, and inline comments helped while reviewing the code. The code in the query graph builder and schema generation for this diff is not trivial.
if parent_is_create { | ||
handle_one_to_one_parent_create(graph, parent_node, parent_relation_field, filter, child_model) | ||
} else { | ||
handle_one_to_one_parent_update(graph, parent_node, parent_relation_field, filter, child_model) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's easier to read than before, so 👍🏾
if parent_relation_field.is_inlined_on_enclosing_model() { | ||
// Inlined on parent | ||
let parent_model = parent_relation_field.model(); | ||
let extractor_model_id = parent_model.primary_identifier(); | ||
let null_record_id = SelectionResult::from(&parent_relation_field.linking_fields()); | ||
// If the relation is inlined on the parent and a filter is applied on the child then it means the update will be done on the parent table. | ||
// Therefore, the filter applied on the child needs to be converted to a "relational" filter so that the connector renders the adequate SQL to join the Child table. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏾
Overview
fixes prisma/prisma#16743
fixes prisma/prisma#16843
This PR fixes a regression introduced by #3443. Specifically, when a nested connect is in a create, we should always attempt to disconnect the already connected record so that the create can succeed.
A couple more things were done:
relation_link_test
macro always inlined the relation on the child side for optional 1-1 relation. This is now fixed, hence why the PR doesn't add another test. This test lives hereextendedWhereUnique
. Note that it's not fixed for MongoDB because the solution might require us to remove it entirely from the api. An issue was created here: Nested disconnect fails on MongoDB with extendedWhereUnique prisma#16869