feat(prisma): run queries through the bound model and merge args - #846
Merged
Merged
Conversation
A model-bound adapter can now run what it serialized, the counterpart
of the typeorm adapter applying its state to the bound query builder:
`findMany(query, options)` pipes execute() into the delegate,
`count(query, options)` reports the pre-pagination total (the where,
never the page window), and `apply(query, options)` returns
`{ data, total, pagination }` in one call, shaped like
`@rapiq/memory`'s applyQuery. On an adapter constructed with explicit
`{ provider, metadata }` the runners raise a typed error; `execute()`
stays the pure serializer either way.
The base-merging rules move out of the adapter into an exported
`mergeArgs(base, override)` helper, since prisma ships no per-call
args composition of its own ($extends intercepts every call
globally): where conditions conjoin, an overriding include joins a
baseline select instead of replacing it, orderBy/take/skip follow the
override and unknown keys (cursor, distinct, ...) pass through.
`execute(query, { base })` is exactly this merge applied to what the
query produced; the impossible-condition root form stays a special
case because a nested empty OR group would be stripped by prisma.
The engine suite runs apply/findMany against the real client,
including a baseline where conjunction.
📝 WalkthroughWalkthroughPrisma adapters now support model-bound ChangesPrisma query execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant PrismaAdapter
participant PrismaDelegate
Caller->>PrismaAdapter: apply(query, options)
PrismaAdapter->>PrismaDelegate: findMany(serialized args)
PrismaAdapter->>PrismaDelegate: count(baseline where)
PrismaDelegate-->>PrismaAdapter: rows and total
PrismaAdapter-->>Caller: data, total, pagination
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/prisma/src/adapter/module.ts (1)
149-194: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturned
paginationdoesn't reflect baseline-suppliedtake/skip.
pagination: { limit, offset }(line 193) is built from the query's own pagination, but the actual args sent to Prisma may carry a differenttake/skipinherited frombaseviamergeArgs(which preservesbase.take/base.skipwhenever the query itself doesn't set them). This contradicts the documented contract onPrismaAdapterOutput.pagination/ApplyOutput.pagination: "the pagination actually applied, e.g. for the response meta block". A caller relying onbaseto supply a default page size will get a metadata block that silently disagrees with what was actually queried.🐛 Proposed fix
return { args: args as ARGS, - pagination: { limit, offset }, + pagination: { limit: args.take, offset: args.skip }, };🤖 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/prisma/src/adapter/module.ts` around lines 149 - 194, Update the returned pagination metadata in the query-building flow around mergeArgs so it reflects the take/skip values actually present in the merged args, including values inherited from base when query.pagination omits them. Preserve explicit query pagination, including 0, and derive the metadata consistently from the final args sent to Prisma.
🧹 Nitpick comments (2)
packages/prisma/test/unit/run.spec.ts (1)
120-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnbound-adapter coverage only exercises
findMany.Given the sync-throw-vs-async-rejection inconsistency flagged in
module.ts(findMany/countthrow synchronously,applyrejects), consider adding equivalent unbound-adapter tests forcountandapply(the latter via.rejects) so both error-surfacing paths are locked in by tests.🤖 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/prisma/test/unit/run.spec.ts` around lines 120 - 129, Extend the unbound-adapter test coverage around PrismaAdapter to include equivalent assertions for count and apply, using the existing FEATURE_UNSUPPORTED error expectation. Keep count’s synchronous throw assertion consistent with findMany, and verify apply through an async rejection assertion with rejects.packages/prisma/src/adapter/merge.ts (1)
27-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Argstyping doesn't support the passthrough contract this function advertises.The docstring promises pass-through of arbitrary keys (
cursor,distinct, ...), butoverride: Args(andT extends Args) has no index signature, soresttypes down to essentially nothing extra. Callers must cast toanyto exercise this documented behavior — as seen in the accompanying test ({ cursor: { id: 5 }, distinct: ['email'] } as any). SincemergeArgsis now exported publicly, consider wideningArgs(e.g. an index signature for unknown keys) so consumers get type-safe passthrough without an escape hatch.🤖 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/prisma/src/adapter/merge.ts` around lines 27 - 38, Update the Args typing used by mergeArgs so arbitrary passthrough keys such as cursor and distinct are accepted without casts. Add an appropriate index signature or equivalent widening to Args, while preserving the existing mergeArgs generic return behavior and known argument fields.
🤖 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/docs/guide/executing-queries.md`:
- Around line 51-56: Update the model-bound adapter example around adapter.apply
so the adapter used for the request is constructed with model: prisma.user
instead of only metadata. Keep the existing query and base scope unchanged, or
introduce a separate model-bound adapter before invoking apply.
In `@packages/docs/packages/prisma.md`:
- Line 56: Update the Prisma documentation sentence describing execute(query, {
base }) and mergeArgs to qualify that they are equivalent for normal argument
merging, except that execute applies additional handling for impossible filters
afterward; replace the unqualified “exactly this merge” wording without changing
the documented merge behavior.
In `@packages/prisma/src/adapter/module.ts`:
- Around line 62-80: Update resolveDelegate so object-form options.model is
accepted only when it exposes a callable findMany method, matching the existing
property-lookup validation; otherwise return undefined so the caller produces
its standard typed AdapterError for binding failures.
- Around line 203-265: Mark the model-bound adapter methods findMany and count
as async so synchronous errors from delegate() become promise rejections,
matching apply’s behavior. Update the unbound-adapter tests for these methods to
assert rejection with .rejects rather than expecting synchronous throws.
---
Outside diff comments:
In `@packages/prisma/src/adapter/module.ts`:
- Around line 149-194: Update the returned pagination metadata in the
query-building flow around mergeArgs so it reflects the take/skip values
actually present in the merged args, including values inherited from base when
query.pagination omits them. Preserve explicit query pagination, including 0,
and derive the metadata consistently from the final args sent to Prisma.
---
Nitpick comments:
In `@packages/prisma/src/adapter/merge.ts`:
- Around line 27-38: Update the Args typing used by mergeArgs so arbitrary
passthrough keys such as cursor and distinct are accepted without casts. Add an
appropriate index signature or equivalent widening to Args, while preserving the
existing mergeArgs generic return behavior and known argument fields.
In `@packages/prisma/test/unit/run.spec.ts`:
- Around line 120-129: Extend the unbound-adapter test coverage around
PrismaAdapter to include equivalent assertions for count and apply, using the
existing FEATURE_UNSUPPORTED error expectation. Keep count’s synchronous throw
assertion consistent with findMany, and verify apply through an async rejection
assertion with rejects.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cf759faa-4532-4d4f-b54f-feba14ebd590
📒 Files selected for processing (9)
packages/docs/guide/executing-queries.mdpackages/docs/packages/prisma.mdpackages/prisma/README.mdpackages/prisma/src/adapter/index.tspackages/prisma/src/adapter/merge.tspackages/prisma/src/adapter/module.tspackages/prisma/src/adapter/types.tspackages/prisma/test/unit/engine.db.spec.tspackages/prisma/test/unit/run.spec.ts
Runner errors now reject consistently: findMany and count are async so an unbound adapter surfaces the typed error as a promise rejection (apply already did), never as a synchronous throw a .catch() would miss. An object passed as `model` without a callable findMany no longer binds silently; the runners keep their typed error instead of a raw TypeError from inside the delegate. Docs: the executing-queries example constructs a model-bound adapter (the previous one was unbound and missing its provider, so apply() could never run), and the mergeArgs equivalence on the package page now names the impossible-condition root form as the one exception.
A bundled rows-plus-total call hides a second query (a count on every request, wanted or not) and pairs the two results without a transaction, so they can be mutually inconsistent under concurrent writes. findMany and count stay as the primitives; an endpoint that wants both composes them, and prisma's own $transaction remains available when the pair must be consistent.
Merged
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
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.
Follow-up to #845, closing the gap against the typeorm adapter: a model-bound
PrismaAdaptercan now run what it serialized, and the args-merge rules become a public helper.Runners
findMany(query, options):execute()piped into the delegate.count(query, options): the pre-pagination total (sees thewhere, including any baseline, never the page window).There is deliberately no bundled rows-plus-total call: it would hide a second query on every request and, without a transaction, could pair mutually inconsistent results. The two primitives compose, and prisma's
$transactionremains available when the pair must be consistent.execute()stays the pure serializer; on an adapter constructed with explicit{ provider, metadata }the runners reject with a typed error, since there is nothing to run against. Runner failures are always promise rejections, never synchronous throws, and an object passed asmodelwithout a callablefindManydoes not bind.mergeArgsPrisma ships no per-call args composition (
$extendsintercepts every call globally), so the merge the adapter always applied to itsbaseoption is now exported:whereconditions are conjoined (AND), an overridingincludejoins a baselineselectinstead of replacing it (a caller-owned projection is never widened),orderBy/take/skipfollow the override, and unknown keys (cursor,distinct, ...) pass through.execute(query, { base })is this merge applied to what the query produced, with the impossible-condition root form ({ OR: [] }) as the one special case, since a nested empty group would be stripped by prisma.Testing
Recording-delegate unit tests (argument shapes per runner, baseline conjunction, typed rejections on unbound or non-runnable bindings) and
mergeArgssemantics, plus engine-backed runs against the real client, including the rows-plus-total composition and a baselinewhereconjunction.