fix(target-postgres): decode native-enum array columns - #30195
fix(target-postgres): decode native-enum array columns#30195StevenMcClankerton wants to merge 2 commits into
Conversation
@prisma/orm-extension-arktype-json
@prisma/orm-extension-middleware-cache
@prisma/orm-extension-paradedb
@prisma/orm-extension-pgvector
@prisma/orm-extension-postgis
@prisma/orm-extension-supabase
@prisma/orm-family-mongo
@prisma/orm-family-sql
@prisma/orm-framework
@prisma/orm-mongo
@prisma/orm-postgres
@prisma/orm-sqlite
@prisma/orm-target-mongo
@prisma/orm-target-postgres
@prisma/orm-target-sqlite
@prisma/orm-toolchain
commit: |
size-limit report 📦
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change preserves descriptor context during codec materialization and casts PostgreSQL native enum-array projections to ChangesEnum-array codec flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The change adds native enum-array decoding support and fixes codec materialization behavior, with targeted and full-suite tests reported passing; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes address issue Full details: Out of Scope Changes checkExplanation The changes remain within scope. The codec regression test, SQL-renderer tests, integration fixture, enum-array read test, and re-enabled enum filter test support the native enum-array decoding fix and its regression coverage. Full details: Docstring CoverageExplanation Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 9 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
A pg.enum(Mood)[] column could not be read: the array-of-enum OID is
allocated per-database, pg-types has no parser for it, and the wire
value arrived as the raw literal `{URGENT,LOW}`, so decoding crashed
before it could report which column failed.
Two independent fixes:
- materializeCodec called descriptor.factory detached from its
receiver (`blindCast<...>(descriptor.factory)(validated)(ctx)`), so
any descriptor whose factory builds its codec via `new XCodec(this)`
produced a codec with `descriptor === undefined`. That codec was
found correctly; only its `id` getter crashed inside
wrapDecodeFailure/wrapEncodeFailure, which is why the error named no
column. Calling factory as a method on descriptor fixes both paths.
- renderProjection/renderReturning now cast a many + pg/enum@1
projection to `::text[]`, putting the column on OID 1009 which
pg-types already parses into a string array. This is a target-owned
fact read from the contract, not a wire-value heuristic, and keeps
array-literal parsing out of the target-agnostic sql-runtime decoder.
Un-skips ports/engines/queries/filters/field-reference/enum-filter,
which was marked it.fails in #29924 without diagnosis: it was this
same decode bug via a native-enum array column.
Fixes #30164.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UcqoY3CKfnubdZt5YQk2Rq
Signed-off-by: Steven McClankerton <tatarintsev@prisma.io>
The suite is not a port of an upstream Prisma test, so it moves to its own directory alongside the relocated harness. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UcqoY3CKfnubdZt5YQk2Rq Signed-off-by: Steven McClankerton <tatarintsev@prisma.io>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Summary
An enum-array column (
pg.enum(Mood)[]) could not be read: the array-of-enum OID is allocated per-database,pg-typeshas no parser for it, the value arrived as the raw literal{URGENT,LOW}, and decoding crashed withTypeError: Cannot read properties of undefined (reading 'codecId').Fixes #30164.
This PR contains two independent changes.
Change A — a framework-wide
this-binding bugmaterializeCodeccalledblindCast<…>(descriptor.factory)(validated)(ctx), detachingfactoryfrom its receiver. Any descriptor whose factory builds its codec vianew XCodec(this)(the standard pattern) therefore produced a codec withdescriptor === undefined.The corrected causal chain is stronger than the issue's own guess: the codec was found; its
idgetter crashed.CodecImpl.get id()isthis.descriptor.codecId(codec.ts:73);decoding.ts:250-259callswrapDecodeFailure, which readscodec.idat:168.encoding.ts:131,150had the identical latent break on the encode side. This is why the error named no column — the good message (Failed to decode column ${table}.${column}…) already existed inwrapDecodeFailure, it just crashed before it could render.Blast radius is small:
this.descriptoris dereferenced at exactly four production sites, none branching or memoising on it. Closure-style factories were already unaffected —PostgresCodecDescriptorAdapterassignsthis.factory = (params) => descriptor.factory(params)(codec-descriptor.ts:139), an arrow that already forwards correctly, so every adapted extension descriptor (pgvector, postgis, arktype-json) was already fine onmain; only directly-declaredCodecDescriptorImplsubclasses were broken. Removing theblindCastis a strict narrowing.Change B — cast enum-array projections to
::text[]projectsNativeEnumArraygatesrenderProjectionandrenderReturningoncodec.codecId === pgEnumDescriptor.codecId && codec.many === true— facts read from the contract, never from the wire value — and appends::text[]to the rendered column. That puts the column on OID 1009, whichpg-typesalready parses into a string array;pg/enum@1'sdecodeis a passthrough.Why this layer. Array-literal parsing does not belong in
sql-runtime/decoding.ts— that package is target-agnostic (no-target-branches), and the postgres target/adapter package is the right home for a postgres wire-format fact.Rejected approaches:
text(OID 25) andvarchar(1043). Verified end-to-end against real Postgres, it corrupted atextcolumn holding'{"a": 1}'into["a", ": 1"]and threwarray dimension not balancedon unbalanced braces — reintroducing exactly the "no column named" diagnostic problem this issue complains about, on a new path.pg_typeOID discovery (queryingtyparrayfor native enum types once per connection) works but adds a bootstrap query that shows up in every exact-query-sequence test in the driver suite (driver.pinned-client-serialization.test.ts,driver.stream-portal-protection.integration.test.ts,driver.prepared.test.ts).The projection cast needs neither: no wire probe, no unregistered-OID guessing, and it never touches
textcolumns.Completeness.
renderProjectionhas one caller (renderSelect), which serves top-level, derived-table, and subquery SELECTs at every nesting depth;renderReturningcovers INSERT/UPDATE/DELETE.includerelations never emit a raw enum array at all — they route throughjsonArrayProjection(codec-descriptor.ts:51-82), which unnests and re-aggregates asjson/jsonb(OIDs 114/3802), so nested/aggregated reads were never affected by this bug.Two shapes are unreachable today but would reintroduce the bug if these surfaces ever grow computed items: a literal-expression projection short-circuits before the cast check, and a non-column-ref
RETURNINGitem skips it. Worth a follow-up if either surface starts accepting computed enum-array expressions.The forced explicit alias in
renderReturningwhen the cast fires is defensive, not load-bearing — Postgres already preserves the column name through a cast in its own column-naming — chosen so the emitted SQL is self-describing.A second, pre-existing reproduction, un-masked
ports/engines/queries/filters/field-reference/enum-filter(enum_filter.test.ts) was markedit.failsin #29924 without anyone diagnosing why. It creates a native-enum array column (enum2: ['a','b']) and hit this exact decode bug. It is nowitand green.While un-skipping it, I removed a duplicate assertion: two
expect(...)calls in that test invoked the identicalreferencedScalarInList(scalar, list, true)under two different labels ('notIn'and'not: { in }'), with no way, given the helper'snegated: booleansignature, to construct a genuinely different negation form. The same copy-paste pair exists verbatim in eight sibling filter ports (bytes_filter,datetime_filter,decimal_filter,bigint_filter,string_filter,int_filter,float_filter, plusenum_filteritself). This PR only cleansenum_filter, ahead of it going green; a sweep of the other eight should be its own follow-up ticket.Testing
test/integrationsuite exits zero: 372 files / 2063 passed | 52 expected fail (2115 total).sql-renderer.enum-array-projection-cast.test.tspins exact rendered SQL for the enum-array SELECT and RETURNING cast, plus four negatives (scalar enum, ordinarytext[], scalartext, no codec) so a future widening of the guard breaks a test instead of silently casting the wrong column.issues-30164-enum-array-decodeport fixture asserts through the full ORM stack that a plainnote: '{"a": 1}'column round-trips as a string beside a correctly-decodedmoodsarray, on both thecreate()RETURNING path and a plain read.materialize-codec.test.tspins Change A directly: aCodecImplsubclass whose descriptor factory doesnew XCodec(this), resolved throughmaterializeCodec, has a working.id.driver-postgresreturns to its untouched 151/151 baseline;temporal-text-parsers.tsis unchanged frommain, and the three exact-query-sequence tests it has were never touched.🤖 Generated with Claude Code
https://claude.ai/code/session_01UcqoY3CKfnubdZt5YQk2Rq
Summary by CodeRabbit
RETURNINGclauses for native enum arrays by applying the appropriate text-array conversion while preserving aliases.