Repository navigation
fix(qc): keep the where filter on updateMany when relations are emulated - #5868
JEONGJAEIK wants to merge 1 commit into
Conversation
With relationMode = "prisma", updateMany read the matching ids and then ran the UPDATE with an empty filter, so a row that stopped matching in between was still updated and counted. Apply the filter to the UPDATE as well, unless the update changes fields referenced by a relation with onUpdate Cascade or SetNull. In that case the emulated actions have already written to the related rows of every pre-read row. The OCC updateMany test now also runs with relation_mode = "prisma" and is no longer excluded on PlanetScale, where it failed because of this.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
Summary by CodeRabbit
WalkthroughThe update-many query graph now retains the original filter in the generated update, except when emulated on-update actions write related records. A helper checks whether Cascade or SetNull actions affect fields in the update arguments. New query-compiler fixtures cover relation-mode Prisma operations and relation actions. The optimistic concurrency test now runs on PlanetScale and excludes D1. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (6 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: preserving the where filter for updateMany when relations are emulated. Description check The description is directly related to the changes. It explains the race condition, implementation, exceptions, tests, and known limitations. Linked Issues check The PR implements #30758in the non-foreign-keyupdateManypath. It clones the pre-read filter and applies it to the UPDATE with the selected IDs. The filter is omitted only when emulatedCascade…Out of Scope Changes check The changes stay within #30758. The update-path helper, relation-mode schema, query fixtures, query-test refactoring, and OCC test updates directly implement or verify the filter-preservation fix. The…Full details: Linked Issues check
-
Explanation
The PR implements
#30758in the non-foreign-keyupdateManypath. It clones the pre-read filter and applies it to the UPDATE with the selected IDs. The filter is omitted only when emulatedCascadeorSetNullactions write related rows, as required by the stated plan.Restrictand other read-only actions retain the filter. Query fixtures cover normal,Restrict,Cascade,SetNull, anddeleteManybehavior. OCC coverage adds the Prisma relation-mode case and removes the obsolete PlanetScale exclusion. The shared update-many path also covers the reportedupdateManyAndReturnand limit variants. The snapshot files are excluded from review, but the source, fixtures, and test changes provide independent evidence of the implementation and coverage.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within
#30758. The update-path helper, relation-mode schema, query fixtures, query-test refactoring, and OCC test updates directly implement or verify the filter-preservation fix. The retained D1 exclusion and the documented Cascade/SetNull limitation do not add unrelated behavior.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (6 skipped: 6 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
✨ Simplify code
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Fixes prisma/orm#30758. Also reported in prisma/orm#21867, and likely prisma/orm#28840 (same
AND 1=1in its log).Problem
With
relationMode = "prisma",update_many_recordsreads the matching ids first and then runs the UPDATE withFilter::empty()(7.10.1 query log, schema and table qualifiers removed):A row that stops matching between the two statements is still updated and counted. So conditional updates that check
count, like the optimistic concurrency control example in the docs, don't work under concurrent writes. WithforeignKeysthe filter is part of the UPDATE (#3175), anddeleteManykeeps it in both modes.Solution
Apply the filter to the UPDATE as well, ANDed with the pre-read ids, the same way
deleteManydoes. This way the UPDATE itself checks the condition on the row it writes.The exception is an update that changes fields referenced by a relation with
onUpdate: CascadeorSetNull. Those emulated actions write to the related rows of every pre-read row before the UPDATE runs, so a filtered UPDATE could skip a row whose related rows were already changed. These updates keep the current plan.RestrictandNoActiononly read the related rows, so the filter is applied for them too. The new helperutils::emulated_on_update_writesmakes the same check asemulate_on_update_cascadeandemulate_on_update_set_null.Tests
query-compiler/query-compiler/tests/queries.rswith arelationMode = "prisma"schema (data/relation-mode-prisma/). I moved the test body into a helper. Existing snapshots are unchanged.update-manyandupdate-many-restrict: the UPDATE now includes the filter. Without the fix both fail: the UPDATE hasAND 1=1instead of the filter.update-many-cascadeandupdate-many-set-null: an update that changes a field referenced withCascadeorSetNullkeeps the current plan.delete-many:deleteManyin the same mode, for comparison.occ_update_many_relation_mode_prisma_test: the existingocc_update_many_testwithrelation_mode = "prisma". On Postgres (make dev-pg-qc) it fails without the fix withleft: 6, right: 1and passes with it. It also passes on better-sqlite3, libsql, MariaDB and SQL Server.occ_update_many_test. The test kit runs PlanetScale withrelationMode = "prisma", and theleft: 6, right: 1failure in the removed comment is this bug. Withmake dev-planetscale-qc, both OCCupdateManytests fail without the fix and pass with it. I left the D1 exclusion as it was.query-engine-testssuite passes on Postgres and PlanetScale with the fix (--test-threads 1).@prisma/client@7.10.1, and ran the reproduction from updateMany with relationMode = "prisma" drops the where filter from the UPDATE, breaking optimistic concurrency orm#30758. WithrelationMode = "prisma", the racingupdateManynow returnscount: 0and stock stays 0 on every run (before:count: 1, stock -1).updateManyAndReturnandupdateManywithlimitalso get the filter.Notes
not,notIn, ...) the UPDATE is no longer chunkable, becauseChunkablecomes from the record filter. On adapters with a bind value limit, a largeupdateManylike that can now go over it. Prisma-modedeleteManyalready behaves this way.CascadeorSetNullstill race. Closing that would mean, for example, running the emulated actions only for rows the UPDATE actually wrote. NestedupdateManyalso reads with the filter and updates withFilter::empty(), in both relation modes. I can look into either in a separate PR if you think it's worth it.The 6.19.x branch has the same code in
query-engine/core. Let me know if you want a backport.