feat: enums#110
Conversation
WalkthroughIntroduces native enum support across the SQL framework, including enum type definitions in schema IR, Postgres migration planning, contract builder API, adapter codecs, and schema verification logic. Adds example app usage with Role enum and validation tests. Changes
Sequence DiagramssequenceDiagram
participant User
participant ContractBuilder
participant SchemaIR
participant PostgresAdapter
participant MigrationPlanner
participant Database
User->>ContractBuilder: define enum('Role', ['USER','ADMIN'])
ContractBuilder->>SchemaIR: store as SqlEnumIR in types
Note over SchemaIR: SqlSchemaIR.enums { Role: { values: [...] } }
User->>PostgresAdapter: build contract
PostgresAdapter->>PostgresAdapter: generate pgEnumCodec
PostgresAdapter->>PostgresAdapter: register enumColumn descriptor
Note over PostgresAdapter: Codec maps string↔string with nativeType
MigrationPlanner->>SchemaIR: extract enums from contract
MigrationPlanner->>Database: verify enum against schema
alt Enum missing
MigrationPlanner->>Database: CREATE TYPE "Role" AS ENUM (...)
else Enum values mismatch (append-only)
MigrationPlanner->>Database: ALTER TYPE "Role" ADD VALUE 'NEWVAL'
else Enum extra (strict mode)
MigrationPlanner->>Database: DROP TYPE "Role"
end
Database->>User: migrations applied
sequenceDiagram
participant SchemaVerifier
participant ContractStorage
participant DatabaseSchema
participant VerificationResult
SchemaVerifier->>ContractStorage: extractEnumsFromContract()
Note over ContractStorage: Prioritize storage.types<br/>Fall back to column typeParams
ContractStorage->>SchemaVerifier: EnumMap { Role: ['USER','ADMIN','MODERATOR'] }
SchemaVerifier->>DatabaseSchema: introspect enums
DatabaseSchema->>SchemaVerifier: enums { Role: ['USER','ADMIN'] }
SchemaVerifier->>SchemaVerifier: compare values
alt Match
SchemaVerifier->>VerificationResult: status: pass
else Append-only mismatch
SchemaVerifier->>VerificationResult: issue: enum_values_mismatch<br/>expected: ['USER','ADMIN','MODERATOR']<br/>actual: ['USER','ADMIN']
else Not append-only (strict)
SchemaVerifier->>VerificationResult: issue: typeMismatch
else Missing in database
SchemaVerifier->>VerificationResult: issue: enum_missing
else Extra in database (strict)
SchemaVerifier->>VerificationResult: issue: extra_enum
end
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Free 📒 Files selected for processing (14)
✏️ Tip: You can disable this entire section by setting Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as off-topic.
This comment was marked as off-topic.
| * const mods = await getUsersByRole('MODERATOR', 5); | ||
| * ``` | ||
| */ | ||
| export async function getUsersByRole(role: 'USER' | 'ADMIN' | 'MODERATOR', limit = 10) { |
There was a problem hiding this comment.
typeof role should be defined as its own type, centralized somewhere.
This should be exported from ../prisma/query.
There was a problem hiding this comment.
Created src/enums/role.ts with ROLE_VALUES, Role type, DEFAULT_ROLE
| const plan = orm.user().create({ | ||
| id: data.id, | ||
| email: data.email, | ||
| role: data.role ?? 'USER', |
There was a problem hiding this comment.
The default value for role should come from a centralized module.
| }) | ||
| .returning(userTable.columns['id']!) | ||
| .build({ params: { id, email, createdAt } }); | ||
| .build({ params: { id, email, role: 'USER', createdAt } }); |
There was a problem hiding this comment.
| }) | ||
| .returning(userTable.columns['id']!) | ||
| .build({ params: { id, email, createdAt } }); | ||
| .build({ params: { id, email, role: 'USER', createdAt } }); |
There was a problem hiding this comment.
| CoreHash extends string | undefined = string | undefined, | ||
| ExtensionPacks extends Record<string, unknown> | undefined = undefined, | ||
| Capabilities extends Record<string, Record<string, boolean>> | undefined = undefined, | ||
| Enums extends Record<string, readonly string[]> = Record<never, never>, |
There was a problem hiding this comment.
In a future iteration, we should allow users to specify whether they want string or number as enum keys.
There was a problem hiding this comment.
The contract should define how enums are persisted.
There was a problem hiding this comment.
Documented this in ADR 155
| const pgEnumCodec = codec<'pg/enum@1', string, string>({ | ||
| typeId: 'pg/enum@1', | ||
| targetTypes: [], // Enum types are dynamically determined by nativeType | ||
| encode: (value) => value, | ||
| decode: (wire) => wire, | ||
| meta: { | ||
| db: { | ||
| sql: { | ||
| postgres: { | ||
| // The actual enum type name is specified per-column via nativeType. | ||
| // This placeholder indicates it's an enum without specifying which one. | ||
| nativeType: 'enum', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
We will want to add new codecs that may be backed by number, not string. That will require an explicit mapping in the contract.
|
|
||
| interface PlannerConfig { | ||
| readonly defaultSchema: string; | ||
| readonly supportsNativeEnums: boolean; |
There was a problem hiding this comment.
This should be a capability, not a boolean.
There was a problem hiding this comment.
However, we're in a Postgres-specific migration planner. This boolean will always be true, so let's drop it.
There was a problem hiding this comment.
Removed this from the planner, kept track of it in ADR 155
| operations.push({ | ||
| id: `enum.${tableName}.${columnName}.check`, | ||
| label: `Add enum check on ${tableName}.${columnName}`, | ||
| summary: `Adds CHECK constraint for enum ${column.nativeType} on ${tableName}.${columnName}`, |
There was a problem hiding this comment.
Keep this idea around for new databases that:
- support CHECK constraints
- do not support native enums
There was a problem hiding this comment.
Kept track of it in ADR 155
| readonly types?: Record<string, StorageTypeInstance>; | ||
| /** | ||
| * Enum type definitions. | ||
| * Enums can be referenced by columns via their nativeType matching the enum name. | ||
| */ | ||
| readonly enums?: Record<string, EnumDefinition>; |
There was a problem hiding this comment.
Why do we need both of these? The whole idea with paramaterized types was to then build enums as a parameterized type, not define a specialized abstraction for them
There was a problem hiding this comment.
Agreed, fixed.
Enums now live in storage.types as regular StorageTypeInstance entries:
- storage.enums.Role = { values: [...] }
+ storage.types.Role = { codecId: 'pg/enum@1', nativeType: 'Role', typeParams: { values: [...] } }I also described this in ADR 155.
| CoreHash extends string | undefined = undefined, | ||
| ExtensionPacks extends Record<string, unknown> | undefined = undefined, | ||
| Capabilities extends Record<string, Record<string, boolean>> | undefined = undefined, | ||
| Enums extends Record<string, readonly string[]> = Record<never, never>, |
There was a problem hiding this comment.
I'm a bit concerned seeing Enums in the framework layer. Are they a necessary abstraction at this layer, because this couples all database families, not just targets, to the enum abstraction
There was a problem hiding this comment.
Right. The Framework layer is now enum-agnostic.
The .enum() method only exists on SqlContractBuilder to add entries to state.types under the hood. There's no coupling with Framework
| [name]: values, | ||
| } as Enums & Record<Name, Values>, | ||
| }; | ||
| // Type assertion needed because base ContractBuilder doesn't know about Enums generic |
There was a problem hiding this comment.
So the base ContractBuilder remains unaware of enums but the base Contract type gained the Enums type parameter?
There was a problem hiding this comment.
I fixed the inconsistency. Both are now unaware:
- ContractBuilder: no Enums generic
- ContractBuilderState: no enums field
- SqlContract:no storage.enums field
SqlContractBuilder.enum('Role', [...]) writes to this.state.types, which already exists. The Contract type remains clean.
wmadden
left a comment
wmadden
left a comment
There was a problem hiding this comment.
High-level
The current implementation adds Postgres enum support, but it does so by breaking the “parameterized type params are opaque/codec-owned” contract and by coupling common SQL logic to enum semantics (and sometimes to a specific codec id). That creates a long-term maintenance hazard and blocks the “strategy/fallback per target (native enum vs CHECK constraint vs other)” goal.
Primary concerns (architectural)
1) extractEnumsFromContract() violates encapsulation
Today it treats any parameterized type with typeParams.values: string[] as an enum (duck typing). That directly contradicts the intended design that typeParams is a black box owned by the codec.
- File:
packages/2-sql/3-tooling/family/src/core/schema-verify/enum-helpers.ts - Impact:
- Any future parameterized codec that uses a
valuesfield (common) could be incorrectly treated as an enum. - Common SQL-family logic is now coupled to a specific param shape, not codec identity/behavior.
- Any future parameterized codec that uses a
2) Common SQL logic is coupled to specific codec ids
Even beyond extraction, there is explicit hardcoding:
- Postgres planner has enum-specific DDL and special-cases
pg/enum@1for column type rendering.- File:
packages/3-targets/3-targets/postgres/src/core/migrations/planner.ts(renderColumnTypecheckscolumn.codecId === 'pg/enum@1')
- File:
- TS contract builder hardcodes enum definitions to
pg/enum@1, making other strategies/targets impossible without rewriting authoring.- File:
packages/2-sql/2-authoring/contract-ts/src/contract-builder.ts(BuildEnumTypeInstanceusescodecId: 'pg/enum@1')
- File:
This is not compatible with the goal of “planner/assembly/common SQL logic must not be coupled to a specific codec id”.
3) ADR 155 valueType is the wrong direction
ADR 155 suggests adding valueType into typeParams to support number-backed enums. This further incentivizes “shared logic interpreting codec params” and deepens the abstraction violation. If params are codec-owned, “number-backed enums” should be represented by a different codec id/version, not a shared valueType convention.
- File:
docs/architecture docs/adrs/ADR 155 - Enum Persistence Strategy.md
4) Schema IR and SchemaIssue encode enums as first-class concepts
Even if enums are implemented via parameterized types, the control plane now has:
SqlSchemaIR.enums(enum-specific)SchemaIssueincludesenum_missing,enum_values_mismatch,extra_enum(enum-specific)
This pushes enum semantics into shared/common layers, contrary to “framework unaware of enums”.
Behavioral issues / mismatches to fix
-
db init policy conflict: the Postgres planner can emit
enum.*.dropasdestructive, butINIT_ADDITIVE_POLICYonly allows additive. There’s a test expecting drop planning even under init policy; that’s inconsistent with “init is additive-only”.- Files:
packages/2-sql/3-tooling/family/src/core/migrations/policies.ts,packages/3-targets/3-targets/postgres/test/migrations/planner.enums.test.ts
- Files:
-
Schema tree output omits enums:
SqlFamilyInstance.toSchemaView()renders tables + extensions but not enums, so introspection output isn’t visible in the CLI tree.- File:
packages/2-sql/3-tooling/family/src/core/instance.ts
- File:
Recommended direction (general solution)
Implement codec-provided control-plane hooks (DDL + verify + introspect semantics) keyed by codec identity, analogous to how parameterized type rendering is already assembled today (types.codecTypes.parameterized via extractParameterizedRenderers()).
- Goal: common SQL logic should:
- group
storage.typesbycodecId - dispatch to codec-owned control hooks
- never inspect
typeParamsstructure (novalues, novalueType, no duck typing) - never special-case codec ids in planners/verifiers
- group
This also enables multiple enum strategies (native enum type vs check constraint) as different codecs, without teaching the core/family about enums.
TML-1818 [PN] Introduce enums
This ticket only concerns with the |
draft
This PR:
Summary by CodeRabbit
Release Notes
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.