fix(postgres-enums): emit enums by native type and address live enum storage by schema coordinate#791
Open
wmadden-electric wants to merge 3 commits into
Open
fix(postgres-enums): emit enums by native type and address live enum storage by schema coordinate#791wmadden-electric wants to merge 3 commits into
wmadden-electric wants to merge 3 commits into
Conversation
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 (15)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughReworks Postgres enum IR: contract→schema options now resolve enum namespaces to DDL schemas, annotations carry enums under ChangesPostgres enum namespace & annotation refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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/emitter
@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: 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/2-sql/9-family/src/core/psl-contract-infer/postgres-type-map.ts`:
- Around line 147-150: The code stores typeInstance.typeParams?.['values'] into
definitions with an unchecked cast (values as string[]); change this to a
runtime guard that ensures values is an array whose every element is a string
before adding to definitions (use Array.isArray(values) && values.every(v =>
typeof v === 'string')), and then insert it as a readonly string[] (e.g., copy
or cast only after the guard) for the Map keyed by nativeType so definitions
remains Map<string, readonly string[]>.
🪄 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: 20eb3cf8-41ee-41f7-9f5b-f23b7143af6b
📒 Files selected for processing (3)
packages/2-sql/9-family/src/core/psl-contract-infer/postgres-type-map.tspackages/2-sql/9-family/test/psl-contract-infer/postgres-type-map.test.tspackages/2-sql/9-family/test/psl-contract-infer/sql-schema-ir-to-psl-ast.test.ts
wmadden-electric
changed the title
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts (1)
636-649:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the unbound schema when merging per-schema annotations.
pg.schemais carried over fromperSchema[0], not from the namespace that resolved fromUNBOUND_NAMESPACE_ID. If an explicit namespace comes first,resolveExistingEnumValues()later looks up unbound enums under the wrong schema and treats existing enums as missing.Suggested fix
+ const unboundIndex = namespaceIds.indexOf(UNBOUND_NAMESPACE_ID); + const unboundSchema = unboundIndex === -1 ? undefined : resolvedSchemas[unboundIndex]; + return { tables: mergedTables, ...ifDefined('annotations', { ...firstAnnotations, pg: { ...firstPg, + ...ifDefined('schema', unboundSchema), ...ifDefined( 'enumTypes', Object.keys(mergedEnumTypes).length > 0 ? mergedEnumTypes : undefined, ), }, }), };🤖 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/control-adapter.ts` around lines 636 - 649, The merged annotations currently take pg.schema from perSchema[0] (firstAnnotations/firstPg) causing unbound enums to be looked up under the wrong schema; modify the merge logic in control-adapter.ts so that when building the returned annotations (the pg object merged from firstPg and enumTypes) you choose pg.schema from the perSchema entry whose namespace equals UNBOUND_NAMESPACE_ID if that entry exists, otherwise fall back to firstPg; update the code paths around perSchema, firstAnnotations, firstPg, mergedEnumTypes and resolveExistingEnumValues to use that selected schema so unbound enums are resolved under the correct schema.
🤖 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.
Outside diff comments:
In `@packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts`:
- Around line 636-649: The merged annotations currently take pg.schema from
perSchema[0] (firstAnnotations/firstPg) causing unbound enums to be looked up
under the wrong schema; modify the merge logic in control-adapter.ts so that
when building the returned annotations (the pg object merged from firstPg and
enumTypes) you choose pg.schema from the perSchema entry whose namespace equals
UNBOUND_NAMESPACE_ID if that entry exists, otherwise fall back to firstPg;
update the code paths around perSchema, firstAnnotations, firstPg,
mergedEnumTypes and resolveExistingEnumValues to use that selected schema so
unbound enums are resolved under the correct schema.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c90dc2b5-9744-4c90-9a7c-250e8de12781
📒 Files selected for processing (15)
packages/2-sql/9-family/src/core/migrations/contract-to-schema-ir.tspackages/2-sql/9-family/src/core/psl-contract-infer/postgres-type-map.tspackages/2-sql/9-family/src/exports/control.tspackages/2-sql/9-family/test/psl-contract-infer/postgres-type-map.test.tspackages/2-sql/9-family/test/psl-contract-infer/print-psl/print-psl.defaults-and-types.test.tspackages/2-sql/9-family/test/psl-contract-infer/print-psl/print-psl.enums.test.tspackages/2-sql/9-family/test/psl-contract-infer/print-psl/print-psl.naming-and-constraints.test.tspackages/2-sql/9-family/test/psl-contract-infer/sql-schema-ir-to-psl-ast.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/enum-planning.tspackages/3-targets/3-targets/postgres/src/exports/control.tspackages/3-targets/3-targets/postgres/src/exports/enum-planning.tspackages/3-targets/3-targets/postgres/test/migrations/enum-collision.test.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/test/control-adapter.test.tstest/integration/test/cross-package/postgres-issue-planner.test.ts
💤 Files with no reviewable changes (1)
- packages/3-targets/3-targets/postgres/src/exports/enum-planning.ts
✅ Files skipped from review due to trivial changes (1)
- packages/2-sql/9-family/src/exports/control.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/2-sql/9-family/test/psl-contract-infer/sql-schema-ir-to-psl-ast.test.ts
…ap key The Postgres control adapter stores introspected enum types in `annotations.pg.storageTypes` keyed by a compound `schema\0nativeType` string (NUL separator). `extractEnumInfo` used that map key directly as the enum's type name, which broke two ways: - The NUL byte leaked into the emitted enum `@@map`, producing PSL with embedded U+0000 that downstream tools treat as binary (#781). - `typeMap.resolve(column.nativeType)` compares against the unqualified `udt_name`, so it never matched the compound key and every enum column fell through to `Unsupported("...")` (#782). Each storage entry already carries the unqualified `nativeType` field, so key `typeNames`/`definitions` off that instead. Names and `@@map` now derive from the clean type name and columns resolve to their enum. Closes #781 Closes #782 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…ate, delete compound key The introspected Schema IR isn't namespace-aware (flat `tables`, no schema on `SqlTableIR`), so live enums couldn't be addressed by the framework's `EntityCoordinate` like everything else. The enum lookup had hand-rolled a `(schema, nativeType)` string key (`enumStorageCompoundKey`) with a NUL separator and stored it in `annotations.pg.storageTypes` — the source of the null-byte leak. Replace that with a structural nesting keyed by the same `(schema, nativeType)` coordinate the contract side uses: enums now live in `annotations.pg.enumTypes` as `Record<schema, Record<nativeType, entry>>`. Non-enum codec types stay flat in `storageTypes`, so SQLite and the vector/decimal paths are untouched. - Delete `enumStorageCompoundKey` and `ENUM_STORAGE_KEY_SEP`. - `EnumStorageKeyResolver(storage, ns, nativeType) -> string` becomes `EnumNamespaceSchemaResolver(storage, ns) -> schema`; the family projector nests by that schema instead of treating an opaque string key. - Introspection writer, the multi-schema merge, and `readExistingEnumValues` all index `enumTypes[schema][nativeType]`. `resolveDdlSchemaForNamespaceStorage` (the legitimate namespace→schema bridge) stays. - `extractEnumInfo` reads the nested map; inferred enum names/`@@map` are unchanged (still derived from the unqualified native type). No persisted artifact changes (these annotations are transient introspection IR, never in contract.json). Cross-schema same-native-type enums stay distinct (enum-collision regression tests still pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
`extractEnumInfo` read `typeParams.values` (typed `unknown`) and stored it with a bare `as string[]`, bypassing validation. Guard every element with a `typeof === 'string'` check so `definitions` (Map<string, readonly string[]>) only holds validated string arrays, and drop the cast. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
wmadden
force-pushed
the
fix/infer-enum-null-byte-map
branch
from
a7bcc91 to
45fe942
Compare
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
2 participants
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.
Problem
contract inferon a Postgres database (e.g. Supabase) produced enum@@mapstrings containing a literal NUL byte (@@map("public\0application_kind")) and emittedUnsupported("...")for columns whose enum type had been inferred (#781, #782).Both traced to one root cause: the Postgres control adapter stored introspected enums in
annotations.pg.storageTypesunder a hand-rolled(schema, nativeType)string key built with a NUL separator (enumStorageCompoundKey). The infer path used that compound key as the enum's type name (NUL leaked into@@map), andtypeMap.resolve(column.nativeType)never matched it (so columns fell through toUnsupported).The deeper issue: the introspected Schema IR isn't namespace-aware (flat
tables, no schema onSqlTableIR), so live enums couldn't be addressed by the framework'sEntityCoordinate(namespaceId+entityKind+entityName) like every other entity. The compound string key was a hack to recover the schema dimension the IR had discarded — and the NUL was a symptom of that hack.Fix
Address live enum storage by the same
(schema, nativeType)coordinate the contract side uses, structurally instead of as a packed string:annotations.pg.enumTypesasRecord<schema, Record<nativeType, entry>>. Non-enum codec types (vector, decimal) stay flat instorageTypes, so SQLite and those paths are untouched.enumStorageCompoundKeyandENUM_STORAGE_KEY_SEP. The family↔target seam changed from "target returns an opaque string key" (EnumStorageKeyResolver) to "target resolves a namespace to its DDL schema" (EnumNamespaceSchemaResolver); the family projector nests by that schema.readExistingEnumValuesall indexenumTypes[schema][nativeType].resolveDdlSchemaForNamespaceStorage(the legitimate namespace→schema bridge) stays.extractEnumInforeads the nested map and keys results by the unqualified native type, so inferred enum names (ApplicationKind) and@@map("application_kind")are clean — no schema prefix, no NUL.No persisted artifact changes: these annotations are transient introspection IR, never written to
contract.json(verified — no NUL byte in any committed.json/.prisma). Cross-schema same-native-type enums stay distinct (the enum-collision regression tests still pass).Tests
postgres-type-map/sql-schema-ir-to-psl-ast: enum columns link to their type and emit a clean, NUL-free@@map;extractEnumInforeads the nestedenumTypesshape.enum-collision(target) andpostgres-issue-planner(integration): cross-schema enum disambiguation preserved under the nested shape.control-adapter: introspection emitsenumTypesnested by schema.family-sql(369),target-postgres(279),extension-pgvector(140) suites pass; lint, typecheck, andlint:depsclean.Closes #781
Closes #782
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor
Tests