tml-2913: native scalar-list query & update operations#910
Open
SevInf wants to merge 6 commits into
Open
Conversation
…ice 3 A]
Promote the prototyped `has` list operation into a live Postgres registry op.
`fns.has(f.tags, x)` now type-checks over a list column and executes as
`x = ANY(tags)` on real Postgres; a wrong-typed element and a scalar receiver
are compile errors.
Necessary unblocks discovered at DoR (both minimal and list-scoped):
- framework operation registry: accept the `{ many: true; elementTraits? }`
self-spec arm (type + register validation), so list-self ops can register.
- SQL emitter: project `many: true` into the storage-table column `.d.ts`
literal, so `Db<Contract>` types a `many` column as a list receiver
(regenerated the scalar-lists fixture .d.ts; contract.json unchanged).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
… 3 B]
Promote the read-side v1 list ops into live Postgres registry ops, mirroring
the `has` pattern (runtime impl in `descriptor-meta.ts`, type mirror in
`operation-types.ts`), each proven to render the right SQL and execute on real
Postgres, with compile-error cases for wrong-typed operands and scalar
receivers:
- `containedBy` (`{{self}} <@ {{arg0}}`) — whole-array operand, bool.
- `overlaps` (`{{self}} && {{arg0}}`) — whole-array operand, bool.
- `length` (`cardinality({{self}})`) — non-null pg/int4.
- `index` (`{{self}}[{{arg0}}]`) — 1-based access, nullable element codec.
Whole-array operands accept a `readonly T[]` literal (lowered to an `ARRAY[...]`
literal) or another `ScalarListExpression<CodecId>`; the element type is tied to
the receiver's `CodecId`, so a wrong-typed element is a compile error. `index`
returns the receiver's element codec (nullable, since out-of-range access is
NULL).
Dropped from this slice: the `contains` (`@>`) op named in the plan collides
with the postgis extension's existing geometry `contains` op. The base postgres
adapter is always a stack contributor, so registering `contains` there
double-registers in any postgres+postgis stack (`Operation "contains" is
already registered`) and breaks `pnpm build`. Reported rather than renamed
against spec or cast around.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Mirror the existing `containedBy` (`<@`) op as `arrayContains` (`@>`, "list contains every element of the argument array") — same whole-array operand shape, same element-type linkage to the receiver `CodecId`, same non-null bool return — differing only in the infix template and the name. Named `arrayContains` rather than `contains`: the op registry is global-by-name and the postgis extension already registers `contains` for geometry, so a base `contains` op would double-register in any postgres+postgis stack. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…et) [slice 3 C] Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
…) [slice 3 D] Add disjoint list-operand overloads to the six comparison builtins (eq/ne/gt/gte/lt/lte) keyed on the `many` discriminant. Each builtin now intersects its untouched scalar signature with a `ListComparison` call signature that accepts a whole-list receiver plus a matching list expression or literal array. Element-trait gating (`equality` for eq/ne, `order` for the ordering ops) is enforced via `CodecIdsWithTrait`, so an ordering op over a list whose element codec lacks `order` is a compile error. Type-level widening only; the runtime already lowers array operands correctly via the shared param path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a ChangesScalar-list operations support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Test as Integration Test
participant Runtime as ORM Runtime
participant Builder as sqlBuilder
participant Adapter as Postgres Adapter
participant DB as PostgreSQL
Test->>Runtime: db.public.Item.create(tags)
Runtime->>Adapter: lower insert with list column
Adapter->>DB: INSERT ... tags
Test->>Builder: build query with has/arrayContains/overlaps
Builder->>Adapter: resolve list operation (elementCodecOf, arrayOperandToExpr)
Adapter->>Adapter: lower to SQL (= ANY, @>, <@, &&, cardinality)
Adapter->>Runtime: emit SQL expression
Runtime->>DB: execute query
DB-->>Runtime: rows
Runtime-->>Test: decoded results
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 |
@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: |
size-limit report 📦
|
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/1-framework/1-core/operations/src/index.ts (1)
44-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winError messages don't reflect the new
manymode.
targetCount === 0still throws"has neither codecId nor traits", andtargetCount > 1still throws"has both codecId and traits"— neither mentionsmany, even though aself: {}(no codecId/traits/many) or a conflicting{ many: true, traits: [...] }self would now hit these same messages. This misleads whoever debugs a failing registration involvingmany. Note the test atoperations-registry.test.ts(Line 122) already hardcodes the stale "both codecId and traits" text, so both need to move together.✏️ Proposed fix for generic messaging
if (targetCount === 0) { - throw new Error(`Operation "${name}" self has neither codecId nor traits`); + throw new Error(`Operation "${name}" self has none of codecId, traits, or many`); } if (targetCount > 1) { - throw new Error(`Operation "${name}" self has both codecId and traits`); + throw new Error(`Operation "${name}" self must have exactly one of codecId, traits, or many`); }🤖 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/1-framework/1-core/operations/src/index.ts` around lines 44 - 61, Update the error handling in register so the messages account for descriptor.self.many as a valid target alongside codecId and traits. In the register name/descriptor flow, change the zero-target and multi-target Error text to mention codecId, traits, and many consistently, and make sure any assertions in operations-registry.test.ts that hardcode the old wording are updated to match the new messages.
🧹 Nitpick comments (5)
packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts (1)
183-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the
elementCodecOfhelper instead of re-inlining its body.
has(Lines 184-189) andindex(Lines 268-273) duplicate the exact codec-resolution block thatelementCodecOfalready encapsulates and thatarrayAppend/arrayRemovereuse. Consolidate to avoid divergence.♻️ Reuse helper in `has`
- const listCodec = codecOf(self); - const elementCodec = - listCodec === undefined - ? undefined - : listCodec.typeParams === undefined - ? { codecId: listCodec.codecId } - : { codecId: listCodec.codecId, typeParams: listCodec.typeParams }; + const elementCodec = elementCodecOf(self);Apply the same substitution in
index(Lines 267-273).🤖 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-targets/6-adapters/postgres/src/core/descriptor-meta.ts` around lines 183 - 189, The codec-resolution logic in has and index is duplicated and should reuse the existing elementCodecOf helper instead of re-inlining its body. Update both locations to call elementCodecOf with the same self argument used in codecOf(self), matching the pattern already used by arrayAppend and arrayRemove, so the codec selection stays centralized in descriptor-meta.ts.packages/2-sql/4-lanes/sql-builder/test/types/scalar-list-ops.types.test-d.ts (1)
91-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing negative test for
eq/newith a non-equality-trait element.All fixtures in
CTcarryequality, so there's no@ts-expect-errorcase provingeq/nereject a list whose element codec lacksequality(only the ordering-trait-missing case is tested viaflags). Not blocking, but would round out the trait-gating coverage symmetrically with the ordering 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/2-sql/4-lanes/sql-builder/test/types/scalar-list-ops.types.test-d.ts` around lines 91 - 127, The scalar-list overload tests in scalar-list-ops.types.test-d.ts are missing a negative case for equality-based comparison on a list whose element codec does not have the equality trait. Extend the existing `fns.eq`/`fns.ne` coverage in the `scalar comparison` / `equality builtins` tests by adding an `@ts-expect-error` assertion using a list fixture whose element type lacks `equality`, mirroring the existing `fns.gt`/`fns.lt` rejection checks for `flags` and keeping the test names and `fns`/`f` symbols aligned with the current suite.packages/2-sql/4-lanes/sql-builder/src/expression.ts (1)
4-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVerbose doc comment on
ListComparison.The design (trait-gated list overload kept disjoint from scalar overload via the
manydiscriminant) is non-trivial and the comment does explain real rationale, but per repo guidelines comments should be avoided when code can express intent directly. Consider trimming to the essential "why" (disjointness viamanydiscriminant) and dropping the mechanical description ofRequiredTraits, which is already conveyed by the type parameter name and usage sites.As per coding guidelines,
**/*.{ts,tsx,js,jsx,mts,mjs}: "Don't add comments if avoidable, prefer code that expresses its intent."🤖 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/2-sql/4-lanes/sql-builder/src/expression.ts` around lines 4 - 35, The doc comment above ListComparison is too verbose for a type that already expresses most of its intent. Trim it down to the minimal rationale needed to understand the overload separation, keeping only the point about the many discriminant making the list and scalar call signatures disjoint, and remove the detailed explanation of RequiredTraits and the mechanical lowering behavior.Source: Coding guidelines
test/integration/test/scalar-lists/scalar-list-mutations.integration.test.ts (2)
168-219: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWhole-array replacement test doesn't assert on generated SQL.
Unlike the
arrayAppend/arrayRemovetest above (which assertscapturedSqlcontainsarray_append(/array_remove(), this test only checks the round-tripped row values. Since this test exists specifically to validate "whole-array replacement via the raw-values path" (per PR objectives), asserting the emitted SQL uses the expected raw-array lowering (rather than only checking end state) would give stronger confidence the intended code path is exercised.🤖 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 `@test/integration/test/scalar-lists/scalar-list-mutations.integration.test.ts` around lines 168 - 219, The whole-array replacement integration test currently only verifies the final row value, so it does not confirm that the raw-array lowering path was used. Update the test named “replaces the whole list column with a raw array literal” to capture the SQL emitted by the `runtime.execute` call that uses `builder.public.item.update({ tags: ['x', 'y'] })`, and assert the generated SQL reflects a raw array replacement rather than `array_append` or `array_remove`. Keep the existing round-trip assertion, but add SQL-level expectations so this test exercises the intended `sqlBuilder`/`rawCodecInferer` path.
105-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame duplicated setup pattern as the sibling read-ops test file.
Both
it()blocks re-implement the schema reset / migrate / runtime+context construction sequence found inorm-list-read.integration.test.ts. Consider factoring a shared helper (in a common test-utils module for thescalar-listssuite) used by both files to avoid the same boilerplate being maintained in two places.🤖 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 `@test/integration/test/scalar-lists/scalar-list-mutations.integration.test.ts` around lines 105 - 219, The two scalar list mutation tests duplicate the same schema reset, migration, and runtime/context setup already used in the sibling read-ops test file. Extract that repeated setup into a shared helper for the scalar-lists suite, then have both `it()` blocks in `scalar-list-mutations.integration.test.ts` call it instead of re-creating the `withClient`, `migrateContract`, `createTestRuntimeFromClient`, `createExecutionContext`, and `sqlBuilder` wiring inline. Keep the helper in a common test-utils module so the setup stays in one place and both mutation tests remain focused on their assertions.
🤖 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-targets/6-adapters/postgres/src/core/descriptor-meta.ts`:
- Around line 148-150: The arrayOperandToExpr path currently renders raw JS
arrays as untyped ARRAY literals, which breaks empty-array operands in
PostgreSQL array operators. Update arrayOperandToExpr and the related renderer
path in descriptor-meta.ts to preserve the receiver’s element codec or emit an
explicit ::type[] cast when creating LiteralExpr.of(operand), so @>, <@, and &&
always get a typed array literal even for empty arrays.
In `@test/integration/test/scalar-lists/orm-list-read.integration.test.ts`:
- Around line 148-411: The four integration tests repeat the same schema reset,
migration, runtime setup, and SQL-capture boilerplate, and the same pattern
appears in the related scalar-list mutation tests. Extract that shared setup
into a helper such as a reset/migrate step plus a setup function that builds the
`captureMiddleware`, `runtime`, `context`, and `capturedSql` for
`createTestRuntimeFromClient`/`createExecutionContext`, then update each `it()`
block to use the shared helper and keep only the test-specific assertions.
In `@test/integration/test/scalar-lists/scalar-list-ops.types.test-d.ts`:
- Around line 155-160: Move the `@ts-expect-error` marker from the comment above
the update callback to the specific id: assignment line in the
db.public.item.update test so it applies to the actual type error on
fns.arrayAppend(f.tags, 'x') and avoids TS2578; use the db.public.item.update,
fns.arrayAppend, and id property locations to place it precisely.
---
Outside diff comments:
In `@packages/1-framework/1-core/operations/src/index.ts`:
- Around line 44-61: Update the error handling in register so the messages
account for descriptor.self.many as a valid target alongside codecId and traits.
In the register name/descriptor flow, change the zero-target and multi-target
Error text to mention codecId, traits, and many consistently, and make sure any
assertions in operations-registry.test.ts that hardcode the old wording are
updated to match the new messages.
---
Nitpick comments:
In `@packages/2-sql/4-lanes/sql-builder/src/expression.ts`:
- Around line 4-35: The doc comment above ListComparison is too verbose for a
type that already expresses most of its intent. Trim it down to the minimal
rationale needed to understand the overload separation, keeping only the point
about the many discriminant making the list and scalar call signatures disjoint,
and remove the detailed explanation of RequiredTraits and the mechanical
lowering behavior.
In
`@packages/2-sql/4-lanes/sql-builder/test/types/scalar-list-ops.types.test-d.ts`:
- Around line 91-127: The scalar-list overload tests in
scalar-list-ops.types.test-d.ts are missing a negative case for equality-based
comparison on a list whose element codec does not have the equality trait.
Extend the existing `fns.eq`/`fns.ne` coverage in the `scalar comparison` /
`equality builtins` tests by adding an `@ts-expect-error` assertion using a list
fixture whose element type lacks `equality`, mirroring the existing
`fns.gt`/`fns.lt` rejection checks for `flags` and keeping the test names and
`fns`/`f` symbols aligned with the current suite.
In `@packages/3-targets/6-adapters/postgres/src/core/descriptor-meta.ts`:
- Around line 183-189: The codec-resolution logic in has and index is duplicated
and should reuse the existing elementCodecOf helper instead of re-inlining its
body. Update both locations to call elementCodecOf with the same self argument
used in codecOf(self), matching the pattern already used by arrayAppend and
arrayRemove, so the codec selection stays centralized in descriptor-meta.ts.
In
`@test/integration/test/scalar-lists/scalar-list-mutations.integration.test.ts`:
- Around line 168-219: The whole-array replacement integration test currently
only verifies the final row value, so it does not confirm that the raw-array
lowering path was used. Update the test named “replaces the whole list column
with a raw array literal” to capture the SQL emitted by the `runtime.execute`
call that uses `builder.public.item.update({ tags: ['x', 'y'] })`, and assert
the generated SQL reflects a raw array replacement rather than `array_append` or
`array_remove`. Keep the existing round-trip assertion, but add SQL-level
expectations so this test exercises the intended `sqlBuilder`/`rawCodecInferer`
path.
- Around line 105-219: The two scalar list mutation tests duplicate the same
schema reset, migration, and runtime/context setup already used in the sibling
read-ops test file. Extract that repeated setup into a shared helper for the
scalar-lists suite, then have both `it()` blocks in
`scalar-list-mutations.integration.test.ts` call it instead of re-creating the
`withClient`, `migrateContract`, `createTestRuntimeFromClient`,
`createExecutionContext`, and `sqlBuilder` wiring inline. Keep the helper in a
common test-utils module so the setup stays in one place and both mutation tests
remain focused on their assertions.
🪄 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: aeb15c2f-8477-403f-8fe6-8cfead5ff0f5
⛔ Files ignored due to path filters (1)
test/integration/test/sql-orm-client/fixtures/scalar-lists/generated/contract.d.tsis excluded by!**/generated/**
📒 Files selected for processing (12)
packages/1-framework/1-core/operations/src/index.tspackages/1-framework/1-core/operations/test/operations-registry.test.tspackages/2-sql/3-tooling/emitter/src/index.tspackages/2-sql/3-tooling/emitter/test/emitter-hook.storage-column-types.test.tspackages/2-sql/4-lanes/relational-core/src/expression.tspackages/2-sql/4-lanes/sql-builder/src/expression.tspackages/2-sql/4-lanes/sql-builder/test/types/scalar-list-ops.types.test-d.tspackages/3-targets/6-adapters/postgres/src/core/descriptor-meta.tspackages/3-targets/6-adapters/postgres/src/types/operation-types.tstest/integration/test/scalar-lists/orm-list-read.integration.test.tstest/integration/test/scalar-lists/scalar-list-mutations.integration.test.tstest/integration/test/scalar-lists/scalar-list-ops.types.test-d.ts
Comment on lines
+148
to
+411
|
|
||
| it( | ||
| 'filters rows through the native `has` membership op, lowering to `= ANY(...)`', | ||
| async () => { | ||
| if (!database) throw new Error('database not initialised'); | ||
|
|
||
| await withClient(database.connectionString, async (client) => { | ||
| await client.query('DROP SCHEMA IF EXISTS public CASCADE'); | ||
| await client.query('CREATE SCHEMA public'); | ||
| await client.query('DROP SCHEMA IF EXISTS prisma_contract CASCADE'); | ||
| }); | ||
| await migrateContract(database.connectionString); | ||
|
|
||
| await withClient(database.connectionString, async (client) => { | ||
| const capturedSql: string[] = []; | ||
| const captureMiddleware: SqlMiddleware = { | ||
| name: 'capture-sql', | ||
| beforeExecute(plan) { | ||
| capturedSql.push(plan.sql); | ||
| }, | ||
| }; | ||
| const runtime = await createTestRuntimeFromClient( | ||
| contract as FrameworkContract<SqlStorage>, | ||
| client, | ||
| { verifyMarker: false, middleware: [captureMiddleware] }, | ||
| ); | ||
|
|
||
| const context = createExecutionContext<Contract>({ | ||
| contract, | ||
| stack: createSqlExecutionStack({ | ||
| target: postgresRuntimeTarget, | ||
| adapter: postgresRuntimeAdapter, | ||
| extensionPacks: [], | ||
| }), | ||
| }); | ||
|
|
||
| const db = orm({ runtime, context }); | ||
| await db.public.Item.create({ id: 1, tags: ['react', 'vue'], scores: [1] }); | ||
| await db.public.Item.create({ id: 2, tags: ['vue', 'svelte'], scores: [2] }); | ||
| await db.public.Item.create({ id: 3, tags: ['svelte'], scores: [3] }); | ||
|
|
||
| const builder = sqlBuilder({ context, rawCodecInferer: postgresRawCodecInferer }); | ||
| const rows = await runtime.execute( | ||
| builder.public.item | ||
| .select('id') | ||
| .where((f, fns) => fns.has(f.tags, 'vue')) | ||
| .orderBy((f) => f.id) | ||
| .build(), | ||
| ); | ||
|
|
||
| expect(capturedSql.some((s) => /= ANY\(/.test(s))).toBe(true); | ||
| expect(rows).toEqual([{ id: 1 }, { id: 2 }]); | ||
| }); | ||
| }, | ||
| timeouts.spinUpPpgDev, | ||
| ); | ||
|
|
||
| it( | ||
| 'filters rows through native array ops (arrayContains @>, containedBy <@, overlaps &&)', | ||
| async () => { | ||
| if (!database) throw new Error('database not initialised'); | ||
|
|
||
| await withClient(database.connectionString, async (client) => { | ||
| await client.query('DROP SCHEMA IF EXISTS public CASCADE'); | ||
| await client.query('CREATE SCHEMA public'); | ||
| await client.query('DROP SCHEMA IF EXISTS prisma_contract CASCADE'); | ||
| }); | ||
| await migrateContract(database.connectionString); | ||
|
|
||
| await withClient(database.connectionString, async (client) => { | ||
| const capturedSql: string[] = []; | ||
| const captureMiddleware: SqlMiddleware = { | ||
| name: 'capture-sql', | ||
| beforeExecute(plan) { | ||
| capturedSql.push(plan.sql); | ||
| }, | ||
| }; | ||
| const runtime = await createTestRuntimeFromClient( | ||
| contract as FrameworkContract<SqlStorage>, | ||
| client, | ||
| { verifyMarker: false, middleware: [captureMiddleware] }, | ||
| ); | ||
|
|
||
| const context = createExecutionContext<Contract>({ | ||
| contract, | ||
| stack: createSqlExecutionStack({ | ||
| target: postgresRuntimeTarget, | ||
| adapter: postgresRuntimeAdapter, | ||
| extensionPacks: [], | ||
| }), | ||
| }); | ||
|
|
||
| const db = orm({ runtime, context }); | ||
| await db.public.Item.create({ id: 1, tags: ['react', 'vue'], scores: [1] }); | ||
| await db.public.Item.create({ id: 2, tags: ['vue', 'svelte'], scores: [2] }); | ||
| await db.public.Item.create({ id: 3, tags: ['svelte'], scores: [3] }); | ||
|
|
||
| const builder = sqlBuilder({ context, rawCodecInferer: postgresRawCodecInferer }); | ||
|
|
||
| const within = await runtime.execute( | ||
| builder.public.item | ||
| .select('id') | ||
| .where((f, fns) => fns.containedBy(f.tags, ['vue', 'svelte'])) | ||
| .orderBy((f) => f.id) | ||
| .build(), | ||
| ); | ||
| expect(capturedSql.some((s) => s.includes('<@'))).toBe(true); | ||
| expect(capturedSql.some((s) => s.includes('ARRAY['))).toBe(true); | ||
| expect(within).toEqual([{ id: 2 }, { id: 3 }]); | ||
|
|
||
| const overlapping = await runtime.execute( | ||
| builder.public.item | ||
| .select('id') | ||
| .where((f, fns) => fns.overlaps(f.tags, ['react'])) | ||
| .orderBy((f) => f.id) | ||
| .build(), | ||
| ); | ||
| expect(capturedSql.some((s) => s.includes('&&'))).toBe(true); | ||
| expect(overlapping).toEqual([{ id: 1 }]); | ||
|
|
||
| const superset = await runtime.execute( | ||
| builder.public.item | ||
| .select('id') | ||
| .where((f, fns) => fns.arrayContains(f.tags, ['svelte'])) | ||
| .orderBy((f) => f.id) | ||
| .build(), | ||
| ); | ||
| expect(capturedSql.some((s) => s.includes('@>'))).toBe(true); | ||
| expect(capturedSql.some((s) => s.includes('ARRAY['))).toBe(true); | ||
| expect(superset).toEqual([{ id: 2 }, { id: 3 }]); | ||
| }); | ||
| }, | ||
| timeouts.spinUpPpgDev, | ||
| ); | ||
|
|
||
| it( | ||
| 'compares whole lists through the eq/gt comparison builtins (FR16)', | ||
| async () => { | ||
| if (!database) throw new Error('database not initialised'); | ||
|
|
||
| await withClient(database.connectionString, async (client) => { | ||
| await client.query('DROP SCHEMA IF EXISTS public CASCADE'); | ||
| await client.query('CREATE SCHEMA public'); | ||
| await client.query('DROP SCHEMA IF EXISTS prisma_contract CASCADE'); | ||
| }); | ||
| await migrateContract(database.connectionString); | ||
|
|
||
| await withClient(database.connectionString, async (client) => { | ||
| const capturedSql: string[] = []; | ||
| const captureMiddleware: SqlMiddleware = { | ||
| name: 'capture-sql', | ||
| beforeExecute(plan) { | ||
| capturedSql.push(plan.sql); | ||
| }, | ||
| }; | ||
| const runtime = await createTestRuntimeFromClient( | ||
| contract as FrameworkContract<SqlStorage>, | ||
| client, | ||
| { verifyMarker: false, middleware: [captureMiddleware] }, | ||
| ); | ||
|
|
||
| const context = createExecutionContext<Contract>({ | ||
| contract, | ||
| stack: createSqlExecutionStack({ | ||
| target: postgresRuntimeTarget, | ||
| adapter: postgresRuntimeAdapter, | ||
| extensionPacks: [], | ||
| }), | ||
| }); | ||
|
|
||
| const db = orm({ runtime, context }); | ||
| await db.public.Item.create({ id: 1, tags: ['react', 'vue'], scores: [1] }); | ||
| await db.public.Item.create({ id: 2, tags: ['svelte'], scores: [2] }); | ||
| await db.public.Item.create({ id: 3, tags: ['react', 'vue'], scores: [3] }); | ||
|
|
||
| const builder = sqlBuilder({ context, rawCodecInferer: postgresRawCodecInferer }); | ||
|
|
||
| const equal = await runtime.execute( | ||
| builder.public.item | ||
| .select('id') | ||
| .where((f, fns) => fns.eq(f.tags, ['react', 'vue'])) | ||
| .orderBy((f) => f.id) | ||
| .build(), | ||
| ); | ||
| expect(capturedSql.some((s) => /"tags" = \$\d+/.test(s))).toBe(true); | ||
| expect(equal).toEqual([{ id: 1 }, { id: 3 }]); | ||
|
|
||
| const greater = await runtime.execute( | ||
| builder.public.item | ||
| .select('id') | ||
| .where((f, fns) => fns.gt(f.tags, ['react'])) | ||
| .orderBy((f) => f.id) | ||
| .build(), | ||
| ); | ||
| expect(capturedSql.some((s) => /"tags" > \$\d+/.test(s))).toBe(true); | ||
| // ['react','vue'] and ['svelte'] both sort lexicographically after ['react'] | ||
| expect(greater).toEqual([{ id: 1 }, { id: 2 }, { id: 3 }]); | ||
| }); | ||
| }, | ||
| timeouts.spinUpPpgDev, | ||
| ); | ||
|
|
||
| it( | ||
| 'computes list length (cardinality) and 1-based nullable element access', | ||
| async () => { | ||
| if (!database) throw new Error('database not initialised'); | ||
|
|
||
| await withClient(database.connectionString, async (client) => { | ||
| await client.query('DROP SCHEMA IF EXISTS public CASCADE'); | ||
| await client.query('CREATE SCHEMA public'); | ||
| await client.query('DROP SCHEMA IF EXISTS prisma_contract CASCADE'); | ||
| }); | ||
| await migrateContract(database.connectionString); | ||
|
|
||
| await withClient(database.connectionString, async (client) => { | ||
| const capturedSql: string[] = []; | ||
| const captureMiddleware: SqlMiddleware = { | ||
| name: 'capture-sql', | ||
| beforeExecute(plan) { | ||
| capturedSql.push(plan.sql); | ||
| }, | ||
| }; | ||
| const runtime = await createTestRuntimeFromClient( | ||
| contract as FrameworkContract<SqlStorage>, | ||
| client, | ||
| { verifyMarker: false, middleware: [captureMiddleware] }, | ||
| ); | ||
|
|
||
| const context = createExecutionContext<Contract>({ | ||
| contract, | ||
| stack: createSqlExecutionStack({ | ||
| target: postgresRuntimeTarget, | ||
| adapter: postgresRuntimeAdapter, | ||
| extensionPacks: [], | ||
| }), | ||
| }); | ||
|
|
||
| const db = orm({ runtime, context }); | ||
| await db.public.Item.create({ id: 1, tags: ['react', 'vue'], scores: [10, 20] }); | ||
| await db.public.Item.create({ id: 2, tags: ['svelte'], scores: [30] }); | ||
|
|
||
| const builder = sqlBuilder({ context, rawCodecInferer: postgresRawCodecInferer }); | ||
| const rows = await runtime.execute( | ||
| builder.public.item | ||
| .select((f, fns) => ({ | ||
| id: f.id, | ||
| len: fns.length(f.tags), | ||
| firstTag: fns.index(f.tags, 1), | ||
| secondScore: fns.index(f.scores, 2), | ||
| })) | ||
| .orderBy((f) => f.id) | ||
| .build(), | ||
| ); | ||
|
|
||
| expect(capturedSql.some((s) => s.includes('cardinality('))).toBe(true); | ||
| expect(capturedSql.some((s) => /\[\$\d+/.test(s))).toBe(true); | ||
| expect(rows).toEqual([ | ||
| { id: 1, len: 2, firstTag: 'react', secondScore: 20 }, | ||
| { id: 2, len: 1, firstTag: 'svelte', secondScore: null }, | ||
| ]); | ||
| }); | ||
| }, | ||
| timeouts.spinUpPpgDev, | ||
| ); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Extract shared setup to remove ~4x duplicated boilerplate.
Each it() block repeats an identical sequence: drop/recreate public schema, drop prisma_contract schema, migrateContract, build capturedSql/captureMiddleware, create runtime, and create context. This is copy-pasted four times (and again in scalar-list-mutations.integration.test.ts). Extracting a shared helper (e.g. a beforeEach that resets the schema/migrates, plus a setupCapturingRuntime(client) helper returning { runtime, context, capturedSql, builder }) would materially cut duplication and make each test focus on its actual assertions.
♻️ Sketch of a shared helper
+async function resetSchema(connectionString: string): Promise<void> {
+ await withClient(connectionString, async (client) => {
+ await client.query('DROP SCHEMA IF EXISTS public CASCADE');
+ await client.query('CREATE SCHEMA public');
+ await client.query('DROP SCHEMA IF EXISTS prisma_contract CASCADE');
+ });
+}
+
+function setupCapturingRuntime(client: Client) {
+ const capturedSql: string[] = [];
+ const captureMiddleware: SqlMiddleware = {
+ name: 'capture-sql',
+ beforeExecute(plan) {
+ capturedSql.push(plan.sql);
+ },
+ };
+ return { capturedSql, captureMiddleware };
+}Then each it() becomes: await resetSchema(...); await migrateContract(...); await withClient(..., async (client) => { const { capturedSql, captureMiddleware } = setupCapturingRuntime(client); ... }).
🤖 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 `@test/integration/test/scalar-lists/orm-list-read.integration.test.ts` around
lines 148 - 411, The four integration tests repeat the same schema reset,
migration, runtime setup, and SQL-capture boilerplate, and the same pattern
appears in the related scalar-list mutation tests. Extract that shared setup
into a helper such as a reset/migrate step plus a setup function that builds the
`captureMiddleware`, `runtime`, `context`, and `capturedSql` for
`createTestRuntimeFromClient`/`createExecutionContext`, then update each `it()`
block to use the shared helper and keep only the test-specific assertions.
Comment on lines
+155
to
+160
| test('a list mutator is rejected for a scalar column', () => { | ||
| // @ts-expect-error -- id is a scalar int column; a text list mutator is not assignable | ||
| db.public.item.update((f, fns) => ({ | ||
| id: fns.arrayAppend(f.tags, 'x'), | ||
| })); | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file snippet =="
sed -n '130,175p' test/integration/test/scalar-lists/scalar-list-ops.types.test-d.ts
echo
echo "== search for update/arrayAppend definitions =="
rg -n "arrayAppend|update\\(" test/integration src packages . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' | head -n 200Repository: prisma/prisma-next
Length of output: 36280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file size =="
wc -l test/integration/test/scalar-lists/scalar-list-ops.types.test-d.ts
echo
echo "== snippet around target =="
nl -ba test/integration/test/scalar-lists/scalar-list-ops.types.test-d.ts | sed -n '145,165p'
echo
echo "== locate relevant symbols =="
rg -n "arrayAppend|arrayPrepend|update\\s*\\(" test/integration test src packages -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' | head -n 200Repository: prisma/prisma-next
Length of output: 312
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to locate the file that defines the database item update shape and array mutators.
for f in $(git ls-files | rg '(\.ts|\.d\.ts|\.tsx)$'); do
if rg -q "arrayAppend|arrayPrepend|update\\s*\\(" "$f"; then
echo "### $f"
rg -n "arrayAppend|arrayPrepend|update\\s*\\(" "$f"
fi
done | head -n 200Repository: prisma/prisma-next
Length of output: 13573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the target test and inspect nearby declarations/types that affect the error location.
echo "== target test snippet =="
nl -ba test/integration/test/scalar-lists/scalar-list-ops.types.test-d.ts | sed -n '145,165p'
echo
echo "== nearby update/mutator declarations =="
rg -n -C 2 "arrayAppend|update\\s*\\(" test/integration/test/scalar-lists/scalar-list-ops.types.test-d.ts test/integration/test -g '!**/node_modules/**' | head -n 200Repository: prisma/prisma-next
Length of output: 222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat >/tmp/ts-expect-error-placement.ts <<'TS'
declare function fn(arg: { id: number }): void;
declare const bad: string;
// case 1: directive above the call line
// `@ts-expect-error`
fn({
id: bad,
});
// case 2: directive above the offending property line
fn({
// `@ts-expect-error`
id: bad,
});
TS
if command -v tsc >/dev/null 2>&1; then
tsc --noEmit --pretty false /tmp/ts-expect-error-placement.ts
else
echo "tsc not available"
fiRepository: prisma/prisma-next
Length of output: 367
Move @ts-expect-error to the id: line
@ts-expect-error only applies to the next line, so the current placement leaves the assignment error on id: fns.arrayAppend(f.tags, 'x') unsuppressed and can also trigger TS2578.
🤖 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 `@test/integration/test/scalar-lists/scalar-list-ops.types.test-d.ts` around
lines 155 - 160, Move the `@ts-expect-error` marker from the comment above the
update callback to the specific id: assignment line in the db.public.item.update
test so it applies to the actual type error on fns.arrayAppend(f.tags, 'x') and
avoids TS2578; use the db.public.item.update, fns.arrayAppend, and id property
locations to place it precisely.
…builtins (FR16) [slice 3 D-redo]
Revert the Dispatch D edit to the shared BuiltinFunctions type: eq/ne/gt/gte/lt/lte are restored to their scalar-only signatures and the sql-builder runtime is untouched. Instead, register six list-comparison ops (eq/ne/gt/lt/gte/lte) in the postgres adapter alongside has/arrayContains, so the Functions = BuiltinFunctions & DeriveExtFunctions intersection widens fns.<op> into an overload: scalar calls resolve via the builtin, whole-list calls via the registry op. Element gating is enforced through CodecIdsWithTrait (equality for eq/ne, order for the ordering ops); the ops have {many: true} self specs so they never surface on the ORM scalar-comparison accessor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
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.
What & why
Slice 3 of native-scalar-arrays (TML-2913). Slices 1–2 made SQL scalar lists first-class in storage/authoring/migration/read (native
text[]/int4[]columns with element-wise codecs). This slice makes them queryable and mutable — the operation surface.Branches off
main(has slices 1 & 2, merged). Independent of the check-constraint-unification PR.What's here
Read predicates & array filters (registry ops on the sql-builder
fnssurface, lowering viaOperationExprtemplates — no new AST):has(x = ANY(tags)) — element membershiparrayContains(@>),containedBy(<@),overlaps(&&) — array filterslength(cardinality),index(tags[i], element-typed, nullable)Update mutators (element-preserving
many: truereturn, on the updatesetcallback):arrayAppend(array_append),arrayRemove(array_remove), set-whole-array (raw-values path)Comparison builtins widened to lists (FR16) —
eq/ne/gt/lt/gte/lteaccept whole-list operands. Type-level only (they already lower toBinaryExpr(op), which Postgres applies to arrays). Added as disjoint overloads keyed on themanydiscriminant so scalar-call inference is untouched (NFR3); gated by the element codec'sequality(eq/ne) /order(ordering) trait.Decisions
arrayContainsfor@>: the natural namecontainsis already registered by the postgis extension (geometry), and the op registry is global-by-name — so the list "contains-all" filter isarrayContains.{ many: true; elementTraits }self-arm, and the SQL emitter now projectsmanyonto the storage-column.d.tsliteral (sof.tagstypes as a list expression). The emitter change is guarded onmany === true— non-list contracts are byte-identical (NFR2; only the one list-column fixture regenerated).Acceptance criteria
where((f,fns)=>fns.has(f.tags,x))executesx = ANY(tags); wrong-typed element and scalar receiver are compile errors. ✅tags String[](array_append/array_remove) and the read returns the mutated list decoded element-wise. ✅fns.eq(f.tags,['a','b'])matches by array equality,fns.gt(f.tags,['a'])by lexicographic order; wrong-typed element and an ordering op on an unorderable-element list are compile errors. ✅Verification
build 68/68 · typecheck 143/143 · lint 82/82 · lint:deps clean · fixtures:check byte-clean · cast ratchet delta 0 · adapter-postgres + sql-builder green · scalar-lists integration green (real Postgres). Runtime ACs proven on embedded Postgres; compile-error ACs via
test-d.tswith load-bearing@ts-expect-error.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes