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

perf: remove redundant read on upsert #4248

Merged
merged 1 commit into from
Sep 18, 2023
Merged

Conversation

Weakky
Copy link
Contributor

@Weakky Weakky commented Sep 18, 2023

Overview

fixes prisma/prisma#5686

When performing an upsert (that's not native), a redundant select was always performed. This PR mitigates that issue.

SQL diff

Before

SELECT `User`.`id` FROM `User` WHERE `User`.`email` = ?;
SELECT `User`.`id` FROM `User` WHERE `User`.`email` = ?;
UPDATE `prisma`.`User` SET `firstName` = ? WHERE `prisma`.`User`.`id` IN (?) AND `prisma`.`User`.`email` = ?;
SELECT `User`.`id` FROM `User` WHERE `User`.`id` = ?;
SELECT `User`.`id` FROM `User` WHERE `User`.`email` = ?;
UPDATE `prisma`.`User` SET `firstName` = ? WHERE `prisma`.`User`.`id` IN (?) AND `prisma`.`User`.`email` = ?;
SELECT `User`.`id` FROM `User` WHERE `User`.`id` = ?;

@Weakky Weakky requested a review from a team as a code owner September 18, 2023 15:30
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 18, 2023

CodSpeed Performance Report

Merging #4248 will not alter performance

Comparing perf/upsert-redundant-select (f948b91) with main (79f2257)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

@miguelff miguelff left a comment

Choose a reason for hiding this comment

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

Neat!

@Weakky Weakky added this to the 5.4.0 milestone Sep 18, 2023
@Weakky Weakky merged commit 237634d into main Sep 18, 2023
30 checks passed
@Weakky Weakky deleted the perf/upsert-redundant-select branch September 18, 2023 16:04
@janpio
Copy link
Contributor

janpio commented Sep 18, 2023

Would be nice if we had a test covering these queries, right?

@miguelff
Copy link
Contributor

We have tests for non-native upserts, I don't think testing the generated queries should be necessary, tho.

@michael-land
Copy link

michael-land commented Oct 4, 2023

@Weakky just curious, Why not use the RETURN statement to avoid an additional SELECT?
from

SELECT `User`.`id` FROM `User` WHERE `User`.`email` = ?;
UPDATE `prisma`.`User` SET `firstName` = ? WHERE `prisma`.`User`.`id` IN (?) AND `prisma`.`User`.`email` = ?;
SELECT `User`.`id` FROM `User` WHERE `User`.`id` = ?;

to

SELECT `User`.`id` FROM `User` WHERE `User`.`email` = ?;
UPDATE `prisma`.`User` SET `firstName` = ? WHERE `prisma`.`User`.`id` IN (?) AND `prisma`.`User`.`email` = ? RETURNING `User`.`id`

In v5.1, the UPDATE already utilizes the RETURN statement."


CleanShot 2023-10-04 at 13 29 17@2x

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.

Unnecessary SELECT may be generated by upsert()
4 participants