feat(queryEngine): add limit argument to updateMany #5110

FGoessler · 2025-01-09T12:13:18Z

This PR adds an optional limit argument to the updateMany call allowing to constraint the max number of affected rows by the update. As requested in prisma/prisma#6957.

It uses a subquery approach similar to deleteMany from #5105.

codspeed-hq · 2025-01-09T12:16:55Z

CodSpeed Performance Report

Merging #5110 will not alter performance

_{Comparing feat/orm-399-limit-updatemany (4107472) with main (a046f87)}

Summary

✅ 11 untouched benchmarks

github-actions · 2025-01-09T12:18:01Z

WASM Query Engine file Size

Engine	This PR	Base branch	Diff
Postgres	2.098MiB	2.097MiB	1.223KiB
Postgres (gzip)	843.463KiB	841.798KiB	1.666KiB
Mysql	2.059MiB	2.058MiB	1.243KiB
Mysql (gzip)	829.007KiB	828.072KiB	958.000B
Sqlite	1.974MiB	1.973MiB	1.243KiB
Sqlite (gzip)	793.513KiB	792.801KiB	729.000B

… deleteMany and updateMany

FGoessler · 2025-01-09T14:19:39Z

...ine/connector-test-kit-rs/query-engine-tests/tests/writes/top_level_mutations/update_many.rs

+          run_query!(
+            &runner,
+            r#"{
+              findManyTestModel(orderBy: { id: asc }) {


ℹ️ Using orderBy here to ensure stability of query output across different DBs. E.g. mongo and postgres returned opposite ordering by default.

I've done the same,
Maybe not in this PR, but it would be nice to be able to do an assertion like assert_has_the_same_elements, which is not easy to do with the current test API, because the results come back as JSON.
That way we would actually be able to cover these cases without using orderBy which is valuable coverage

FGoessler · 2025-01-09T14:21:25Z

query-engine/connectors/sql-query-connector/src/limit.rs

+use quaint::ast::*;
+use query_structure::*;
+
+pub(crate) fn wrap_with_limit_subquery_if_needed<'a>(


ℹ️ Using the same logic to create a subquery for updates and deletes => centralized that logic here.

🤔 Happy to learn whether we have a better place for such shared logic. :)

FGoessler · 2025-01-09T14:24:22Z

query-engine/core/src/query_graph_builder/write/update.rs

@@ -342,3 +358,9 @@ fn can_use_atomic_update(

    true
 }
+
+pub struct UpdateManyRecordNodeOptionals<'a> {


ℹ️ The linter was complaining about too many params on the update_many_record_node method so I extracted the most recently introduced optional params into this param object.

🤔 Happy to learn if you have a stylistic better or more idiomatic approach for this. :)

I think it's okay, but a slightly more idiomatic approach would be to refactor update_many_record_node to a builder pattern (something like UpdateManyRecordNodeBuilder::new(graph, query_schema, filter, model, data_map).limit(1).build()).

FGoessler · 2025-01-09T14:25:53Z

query-engine/core/src/query_graph_builder/write/utils.rs

🤔 Am not 100% sure about the methods in this file but since it mentions emulate and cascade I assume this is not really relevant to leverage a limit param.

query-engine/connectors/mongodb-query-connector/src/root_queries/write.rs

query-engine/connectors/sql-query-connector/src/database/operations/write.rs

jacek-prisma · 2025-01-09T14:45:46Z

query-engine/connectors/sql-query-connector/src/database/operations/update.rs

@@ -136,7 +145,7 @@ pub(super) async fn update_many_from_ids_and_filter(

    let updates = {
        let update = build_update_and_set_query(model, args, selected_fields, ctx);
-        let ids: Vec<&SelectionResult> = ids.iter().collect();
+        let ids: Vec<&SelectionResult> = ids.iter().take(limit.unwrap_or(i64::MAX) as usize).collect();


i64::MAX as usize will overflow on platforms where pointers are less than 8 bytes, I think we compile this to WASM where this would overflow

Correct and I am aware. We do these kind of typecasts unfortunately all over the place. Briefly discussed this also with @aqrln earlier this week in the scope of the deleteMany changes. Not sure how we could best fix universally. Ideas?

I don't know how common of an issue this is in the codebase, but in the two particular cases in this PR you can avoid using i64::MAX:
ids.iter().take(limit.map(|i| usize::try_from(i).unwrap()).unwrap_or(ids.len()))

Also this will not technically "overflow" in an unsafe way (thanks rust 👋 ) but just cap out defacto u32::MAX should the provided limit be larger.

as casts in Rust do overflow (or a better term might be truncate), for example 4294967296u64 as u32 will give you 0

this one is an interesting case though, because casting a bigger MAX to smaller MAX will work fine because truncation will always produce the max value of the smaller type

I'd probably avoid the conversion-through-truncation here, but it's not a big deal, idk how others feel

Updated this with some early sanity checks and user facing error messages in this commit: d8c2f27

jacek-prisma · 2025-01-09T14:46:17Z

query-engine/connectors/mongodb-query-connector/src/root_queries/write.rs

@@ -160,6 +161,7 @@ pub async fn update_records<'conn>(
    let ids: Vec<Bson> = if let Some(selectors) = record_filter.selectors {
        selectors
            .into_iter()
+            .take(limit.unwrap_or(i64::MAX) as usize)


same as https://github.com/prisma/prisma-engines/pull/5110/files#r1908939680

...ine/connector-test-kit-rs/query-engine-tests/tests/writes/top_level_mutations/update_many.rs

query-engine/core/src/query_graph_builder/write/delete.rs

query-engine/core/src/query_graph_builder/write/update.rs

FGoessler added 3 commits January 9, 2025 10:15

feat(updateMany): add test for limit in updateMany

e3360b0

feat(updateMany): implement limit for updateMany

1a7f1e9

docu(updateMany): update comments

f937b9a

FGoessler added 3 commits January 9, 2025 13:31

fix: update snapshots

bedbb5c

refactor: solve too many method args by introducing arg object

d1f8fae

refactor: extract subquery creation to avoid code duplication between…

0a4637f

… deleteMany and updateMany

FGoessler force-pushed the feat/orm-399-limit-updatemany branch from ee23587 to 0a4637f Compare January 9, 2025 13:04

fix(tests): update snapshot test with added limit param

107013a

FGoessler commented Jan 9, 2025

View reviewed changes

FGoessler marked this pull request as ready for review January 9, 2025 14:27

FGoessler requested a review from a team as a code owner January 9, 2025 14:27

FGoessler requested review from jacek-prisma and removed request for a team January 9, 2025 14:27

jacek-prisma reviewed Jan 9, 2025

View reviewed changes

query-engine/connectors/mongodb-query-connector/src/root_queries/write.rs Outdated Show resolved Hide resolved

jacek-prisma reviewed Jan 9, 2025

View reviewed changes

query-engine/connectors/sql-query-connector/src/database/operations/write.rs Outdated Show resolved Hide resolved

jacek-prisma reviewed Jan 9, 2025

View reviewed changes

FGoessler added 2 commits January 9, 2025 16:35

refactor: improve method signature semantics

0473645

cleanup: cleanup leftover log statement

63e697e

aqrln added this to the 6.3.0 milestone Jan 10, 2025

FGoessler added 3 commits January 10, 2025 14:29

refactor: validate provided limit value and prevent overflows

d8c2f27

style: fix linter issues

038f4d3

fix: linter

0120366

jacek-prisma reviewed Jan 10, 2025

View reviewed changes

...ine/connector-test-kit-rs/query-engine-tests/tests/writes/top_level_mutations/update_many.rs Show resolved Hide resolved

jacek-prisma approved these changes Jan 10, 2025

View reviewed changes

aqrln reviewed Jan 10, 2025

View reviewed changes

query-engine/core/src/query_graph_builder/write/delete.rs Outdated Show resolved Hide resolved

aqrln reviewed Jan 10, 2025

View reviewed changes

query-engine/core/src/query_graph_builder/write/update.rs Outdated Show resolved Hide resolved

FGoessler added 3 commits January 10, 2025 16:57

refactor: simplify error handling syntax

4966e5f

chore: add tests for negative limit

d8523a0

Merge branch 'main' into feat/orm-399-limit-updatemany

4107472

FGoessler merged commit 8d21d25 into main Jan 13, 2025
367 checks passed

FGoessler deleted the feat/orm-399-limit-updatemany branch January 13, 2025 08:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(queryEngine): add limit argument to updateMany #5110

feat(queryEngine): add limit argument to updateMany #5110

FGoessler commented Jan 9, 2025

codspeed-hq bot commented Jan 9, 2025 •

edited

Loading

github-actions bot commented Jan 9, 2025 •

edited

Loading

FGoessler Jan 9, 2025

jacek-prisma Jan 9, 2025 •

edited

Loading

FGoessler Jan 9, 2025 •

edited

Loading

FGoessler Jan 9, 2025 •

edited

Loading

aqrln Jan 10, 2025 •

edited

Loading

FGoessler Jan 9, 2025 •

edited

Loading

jacek-prisma Jan 9, 2025

FGoessler Jan 9, 2025

jacek-prisma Jan 9, 2025 •

edited

Loading

FGoessler Jan 9, 2025

jacek-prisma Jan 9, 2025 •

edited

Loading

jacek-prisma Jan 9, 2025

jacek-prisma Jan 9, 2025

FGoessler Jan 10, 2025

jacek-prisma Jan 9, 2025

feat(queryEngine): add limit argument to updateMany #5110

feat(queryEngine): add limit argument to updateMany #5110

Conversation

FGoessler commented Jan 9, 2025

codspeed-hq bot commented Jan 9, 2025 • edited Loading

Merging #5110 will not alter performance

Summary

github-actions bot commented Jan 9, 2025 • edited Loading

WASM Query Engine file Size

Choose a reason for hiding this comment

jacek-prisma Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

FGoessler Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

FGoessler Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

aqrln Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

FGoessler Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacek-prisma Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacek-prisma Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codspeed-hq bot commented Jan 9, 2025 •

edited

Loading

github-actions bot commented Jan 9, 2025 •

edited

Loading

jacek-prisma Jan 9, 2025 •

edited

Loading

FGoessler Jan 9, 2025 •

edited

Loading

FGoessler Jan 9, 2025 •

edited

Loading

aqrln Jan 10, 2025 •

edited

Loading

FGoessler Jan 9, 2025 •

edited

Loading

jacek-prisma Jan 9, 2025 •

edited

Loading

jacek-prisma Jan 9, 2025 •

edited

Loading