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

fix: noop one2m nested connect #3679

Merged
merged 1 commit into from
Feb 8, 2023
Merged

fix: noop one2m nested connect #3679

merged 1 commit into from
Feb 8, 2023

Conversation

Weakky
Copy link
Contributor

@Weakky Weakky commented Feb 7, 2023

Overview

fixes prisma/prisma#17103

When performing a noop update on MySQL & MongoDB, the returned count of "affected rows" by the updateMany mutation was 0. This was breaking nested connect mutations which ensures the mutation has happened by relying on this count.

if let QueryResult::Count(c) = query_result {
if c != &expected_id_count {
return Err(QueryGraphBuilderError::RecordNotFound(format!(
"Expected {expected_id_count} records to be connected after connect operation on one-to-many relation '{relation_name}', found {c}.",
)));
}
}

This PR does the following:

  • Updates mysql_async (fix: pass CLIENT_FOUND_ROWS mysql_async#1) so that it passes the CLIENT_FOUND_ROWS flag. This changes the behavior of the affected count returned for mutations (UPDATE/INSERT etc). It makes MySQL return the FOUND rows instead of the AFFECTED rows.
  • Updates the updateMany connector logic on MongoDB to rely on the matched_count instead of the modified_count
  • Adds a regression test

Note that relying on the "matched count" (found rows) instead of the "modified count" (affected rows) does not break OCC because it still would return 0 if the filter doesn't match any row (because the "discriminant" field (eg: version field) got updated by another update).

It does change what updateMany and deleteMany now returns for MySQL. We believe the only "meaningful" count that can be used in an application to do any kind of logic is 0 though. Since 0 will still be returned in useful cases, we don't believe this is a breaking change.

When performing a noop update on MySQL & MongoDB, the returned "affected count" by the updateMany mutation was 0. This was breaking nested connect mutations which expect the mutation to have happened. MySQL & MongoDB were affected by this.

This commit updates mysql_async (prisma/mysql_async#1) so that it passes the CLIENT_FOUND_ROWS flag. This changes the behaviour of the affected count returned for mutations (UPDATE/INSERT etc). It makes MySQL return the FOUND rows instead of the AFFECTED rows.

It also updates the logic on MongoDB to rely on the `matched_count` instead of the `modified_count`.
@Weakky Weakky requested a review from a team February 7, 2023 15:43
@Weakky Weakky changed the title fix: idempotent nested connect fix: idempotent one2m nested connect Feb 7, 2023
@Weakky Weakky changed the title fix: idempotent one2m nested connect fix: noop one2m nested connect Feb 7, 2023
Copy link
Contributor

@tomhoule tomhoule left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very neat!

@Weakky Weakky merged commit 015eacc into main Feb 8, 2023
@Weakky Weakky deleted the fix/regression-prisma-17103 branch February 8, 2023 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error received when connecting items in an existing one-to-many relationship on MySQL
2 participants