TML-2979: Scope MTI variant predicates in count writes#940
Open
tensordreams wants to merge 1 commit into
Open
Conversation
Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
Contributor
📝 WalkthroughWalkthroughUpdate/delete count operations now propagate variantName and modelName context through Collection methods into compileUpdateCount/compileDeleteCount. A new WHERE-building helper generates EXISTS-based correlated subqueries for multi-table-inheritance (mti) polymorphism, replacing plain filter combination. New tests validate this variant-scoped behavior. ChangesMTI-aware count mutation scoping
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
size-limit report 📦
|
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/3-extensions/sql-orm-client/src/query-plan-mutations.ts`:
- Around line 390-400: Thread MTI context through
`#executeDeleteReturningWithIncludes` and the delete-all include path:
`deleteAll()` is still calling `compileDeleteCount()` without `variantName` and
`modelName`, which lets MTI deletes bypass the `EXISTS` filter. Update the call
chain so `variantName`/`modelName` are passed into `compileDeleteCount()` from
`#executeDeleteReturningWithIncludes`, and ensure the delete mutation continues
to route through `buildCountMutationWhere()` with the MTI predicates intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 88d9cf7a-387e-45b8-a54a-9d2af99a5a0c
📒 Files selected for processing (3)
packages/3-extensions/sql-orm-client/src/collection.tspackages/3-extensions/sql-orm-client/src/query-plan-mutations.tspackages/3-extensions/sql-orm-client/test/collection-variant.test.ts
Comment on lines
+390
to
+400
| variantName?: string | undefined, | ||
| modelName?: string | undefined, | ||
| ): SqlQueryPlan<Record<string, unknown>> { | ||
| const where = combineWhereExprs(filters); | ||
| const where = buildCountMutationWhere( | ||
| contract, | ||
| namespaceId, | ||
| tableName, | ||
| filters, | ||
| variantName, | ||
| modelName, | ||
| ); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining compileDeleteCount callers pass variant/model context when collection state is available.
rg -n -C4 '\bcompileDeleteCount\s*\(' packages/3-extensions/sql-orm-client/srcRepository: prisma/prisma-next
Length of output: 3559
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '385,430p' packages/3-extensions/sql-orm-client/src/query-plan-mutations.ts
printf '\n---\n'
sed -n '2068,2165p' packages/3-extensions/sql-orm-client/src/collection.ts
printf '\n---\n'
sed -n '1028,1042p' packages/3-extensions/sql-orm-client/src/mutation-executor.tsRepository: prisma/prisma-next
Length of output: 4573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' packages/3-extensions/sql-orm-client/src/query-plan-mutations.ts
printf '\n---\n'
rg -n -C3 '\bdeleteAll\b|`#executeDeleteReturningWithIncludes`|compileDeleteCount\(' packages/3-extensions/sql-orm-client/src/collection.ts packages/3-extensions/sql-orm-client/src/mutation-executor.tsRepository: prisma/prisma-next
Length of output: 14610
Thread MTI context through #executeDeleteReturningWithIncludes.
deleteAll() with includes still calls compileDeleteCount() without variantName/modelName, so MTI deletes can skip the EXISTS predicate and apply the outer DELETE directly to variant-table columns.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/3-extensions/sql-orm-client/src/query-plan-mutations.ts` around
lines 390 - 400, Thread MTI context through `#executeDeleteReturningWithIncludes`
and the delete-all include path: `deleteAll()` is still calling
`compileDeleteCount()` without `variantName` and `modelName`, which lets MTI
deletes bypass the `EXISTS` filter. Update the call chain so
`variantName`/`modelName` are passed into `compileDeleteCount()` from
`#executeDeleteReturningWithIncludes`, and ensure the delete mutation continues
to route through `buildCountMutationWhere()` with the MTI predicates intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue
Refs TML-2979
Summary
updateCountanddeleteCountnow compile MTI variant-narrowed predicates through a correlated joined subquery instead of placing variant-table references directly in the outer write. The matching count read also keeps variant context so it counts the same joined row set that the write targets.At a glance
Before this change, the accepted predicate could compile into a bare
UPDATE ... WHERE features.priority > ...withoutfeaturesin scope.Decision
This PR ships correlated-subquery planning for MTI variant-narrowed
updateCountanddeleteCount.The count read propagates
variantNameandmodelNameintocompileSelect, so the existing read planner joins the selected MTI table. The count write detects MTI variant narrowing and wraps the original filters inWHERE EXISTS (...), where the inner query aliases the base table, joins the variant table, and correlates back to the outer write by primary key.Reviewer notes
compileUpdateCount/compileDeleteCount; non-variant and STI paths intentionally keep the old plain-filter shape.ascasts; the package lint still prints existing no-bare-cast infos from older production files, but it exits successfully.How it fits together
Collection.updateCountandCollection.deleteCountbuild a matching primary-key read before the write. That read now carriesvariantNameandmodelNameintocompileSelect, allowing the normal MTI join machinery to joinfeatures.query-plan-mutationsaddsbuildCountMutationWhere, which only switches behavior when both a selected variant and model name identify an MTI variant.features.*references now live inside the joined subquery scope.Behavior changes & evidence
MTI scalar predicates in count writes are executable:
.variant('Feature').where((task) => task.priority.gt(1)).updateCount(...)now scopesfeatures.prioritythrough a joined subquery.Variant-declared relation predicates in count writes are executable:
.variant('Feature').where((task) => task.assignee.some()).deleteCount()now scopesfeatures.assignee_idthrough the same joined subquery.Compatibility / migration / risk
No public API or extension-author migration change. The SQL shape changes only for MTI variant-narrowed
updateCountanddeleteCount; plain, non-variant, and STI count writes keep the existing direct WHERE shape.Testing performed
pnpm --filter @prisma-next/sql-orm-client typecheckpnpm --filter @prisma-next/sql-orm-client test -- collection-variant.test.tspnpm --filter @prisma-next/sql-orm-client test -- query-plan-mutations.test.tspnpm --filter @prisma-next/sql-orm-client lintpnpm check:upgrade-coverage --mode prgit diff --checkSkill update
n/a — internal bug fix; no user-facing commands, flags, public TypeScript API, error codes, or glossary terms changed.
Alternatives considered
UPDATE ... FROM/ joined delete support to the mutation AST: this would make the join explicit in the write AST, but it broadens the shared mutation AST and renderer surface for a narrow bug fix. The correlated subquery fits the existing AST.Checklist
git commit -s) per the DCO. The DCO status check will block merge if any commit is missing aSigned-off-by:trailer.n/aif the change is doc-only / refactor with no behavioural delta).TML-NNNN: <sentence-case title>form (Linear ticket prefix + concise title naming the concrete deliverable). See.claude/skills/create-pr/SKILL.mdfor the full convention.n/a — internal only).Notes for the reviewer
The branch intentionally does not add an extension-author upgrade note;
pnpm check:upgrade-coverage --mode prpasses and this is an internal planner bug fix.Summary by CodeRabbit