-
Notifications
You must be signed in to change notification settings - Fork 235
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: noop update from selectors should work #4092
Conversation
CodSpeed Performance ReportMerging #4092 will not alter performanceComparing Summary
|
Self::UpdateRecord(UpdateRecord::WithExplicitSelection(ur)) => { | ||
ur.selected_fields.is_superset_of(field_selection) | ||
} | ||
Self::UpdateRecord(UpdateRecord::WithImplicitSelection(ur)) => { | ||
ur.selected_fields().is_superset_of(field_selection) | ||
} | ||
Self::UpdateRecord(UpdateRecord::WithoutSelection(_)) => returns_id, |
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 a bit out of scope for this PR.
This method, WriteQuery::returns
, is used to compute reload nodes by checking whether a node's field selection fulfills its children data dependencies. It had not been updated after making UpdateRecord
nodes return an arbitrary selection set. This means that some reload nodes might've not been added in cases it's needed.
let filter = build_update_one_filter(record_filter); | ||
|
||
return get_single_record(conn, model, &filter, &selected_fields, &[], ctx).await; |
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 actual fix. The filter generated for the read in the case of a noop update now takes into account the whole RecordFilter
, thus rendering a filter for selectors too if any are present.
Overview
Follow-up to #4084. Fixes a regression for 1-1 update when relationMode=prisma.
The PR fixes two things:
relationMode=prisma
, we read the record to update before doing the actual update to compute whether some referential actions will need to be added to the graph. When doing this additional read, we pass the read ids down to the update operation so that we save reading the ids again when the connector does not supportUpdateReturning
. These ids are passed asRecordFilter.selectors
. However, the selectors were not considered when doing a simple read for a noop update. Therefore, the update node wasn't returning the correct ids, leading to failed operations later on.WriteArgs::returns
is now updated to take into account the fact that update nodes can now return more than just the primary identifier of a model. While we haven't noticed any bug, it could've resulted in missing reload nodes when the child of a node is requesting data dependencies that aren't fulfilled by the parent's selection set.See https://prisma-company.slack.com/archives/C058VM009HT/p1690562457715389