TML-2867: codec-routed DDL default literals — marker/ledger on the typed path#794
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigration planning and execution now accept promise-valued operations and SqlExecuteRequest lowering across framework, SQL family, targets, adapters, tests, and upgrade docs. ChangesAsync migration lowering rollout
Sequence Diagram(s)sequenceDiagram
participant Client
participant Planner
participant RenderableMigration
participant ControlAdapter
participant Runner
Client->>Planner: request plan
Planner->>RenderableMigration: request operations (calls)
RenderableMigration->>ControlAdapter: OpFactoryCall.toOp(lowerer?)
ControlAdapter-->>RenderableMigration: SqlExecuteRequest or Promise(SqlExecuteRequest)
RenderableMigration-->>Planner: operations[] (may include Promise)
Planner->>Runner: plan (with operations)
Runner->>Runner: await Promise.all(plan.operations) -> planOps
Runner->>Runner: enforcePolicy(planOps) / applyPlan(planOps) / recordLedger(planOps.length)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
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/3-targets/6-adapters/sqlite/test/migrations/render-typescript.roundtrip.test.ts (1)
233-233:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing adapter argument and await for
renderOps.Line 233 calls
renderOps(calls)without an adapter argument and withoutawait Promise.all(...), while line 147 correctly callsawait Promise.all(renderOps(calls, testAdapter)). This inconsistency may cause the test to fail or produce incorrect results ifrenderOpsnow requires an adapter and returns promises.🔧 Proposed fix
- const expected = JSON.parse(JSON.stringify(renderOps(calls))); + const expected = JSON.parse(JSON.stringify(await Promise.all(renderOps(calls, testAdapter))));🤖 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/sqlite/test/migrations/render-typescript.roundtrip.test.ts` at line 233, The test currently calls renderOps(calls) without the adapter and without awaiting the async results; update the assignment to pass the adapter and await all promises, i.e. replace the call to renderOps(calls) with await Promise.all(renderOps(calls, testAdapter)) (so the variable expected is built from the resolved results), ensuring you reference the existing renderOps function, the calls test input and the testAdapter variable used earlier in the file.
🧹 Nitpick comments (3)
packages/2-sql/9-family/test/field-event-planner.test.ts (1)
155-157: ⚡ Quick winUse async materialization instead of casting
toOp()results.At Line 155, Line 192, Line 389, Line 439, and Line 563,
(o.toOp() as MigrationPlanOperation).idsidesteps the new async-capable contract. Prefer awaitingtoOp()so these assertions stay valid for both sync and async ops.♻️ Suggested pattern
- expect(ops.map((o) => (o.toOp() as MigrationPlanOperation).id)).toEqual([...]); + const opIds = await Promise.all(ops.map(async (o) => (await o.toOp()).id)); + expect(opIds).toEqual([...]);Also applies to: 192-194, 389-393, 439-443, 563-568
🤖 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/9-family/test/field-event-planner.test.ts` around lines 155 - 157, The test is casting o.toOp() to MigrationPlanOperation synchronously; instead await the async-capable toOp() and then read .id so the assertion supports both sync and async ops — update places using (o.toOp() as MigrationPlanOperation).id (e.g., in the ops mapping assertions) to first materialize toOp() for each op (using Promise.all over ops.map(...) or an equivalent async map) and then assert against the resulting .id values; target the toOp() call and MigrationPlanOperation usage in these test assertions.packages/3-targets/3-targets/postgres/test/migrations/render-ops.test.ts (1)
32-36: 💤 Low valueConsider adding test coverage for async error path.
The current test validates synchronous error throwing when an op has the wrong target. Since
renderOpsnow returns(Op | Promise<Op>)[], consider adding a companion test that verifies the error path when an async op (returningPromise<Op>) has a mismatched target.Example:
it('rejects when an async call materialises an op for a different target', async () => { const asyncCall = { ...makeCall('sqlite', 'table.users.create', 'createTable'), toOp: async () => makeCall('sqlite', 'table.users.create', 'createTable').toOp(), } as unknown as OpFactoryCall; await expect(Promise.all(renderOps([asyncCall]))).rejects.toThrow( /expected postgres op.+target\.id="sqlite"/, ); });🤖 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/3-targets/postgres/test/migrations/render-ops.test.ts` around lines 32 - 36, The test suite only checks synchronous error throwing for renderOps when an op targets the wrong engine; add a companion async test that constructs an OpFactoryCall-like object (use makeCall('sqlite', 'table.users.create', 'createTable') as base), override its toOp to return a Promise resolving to the wrong-target Op, invoke renderOps with that async call and assert the Promise.all(renderOps([...])) rejects with the same regex used in the sync test; reference renderOps, makeCall, OpFactoryCall and toOp when locating where to add the new async test.packages/3-targets/3-targets/postgres/src/core/migrations/runner.ts (1)
651-653: 💤 Low valueMinor: Inconsistent error message formatting.
The error message uses template literal interpolation for
planOpsLengthbut direct string concatenation fortotalEdgeOps. For consistency, both should use the same style.♻️ Suggested consistency fix
throw new Error( - `Ledger write: plan.operations length (${planOpsLength}) does not match sum of migrationEdges operationCount (${totalEdgeOps})`, + `Ledger write: plan.operations length (${planOpsLength}) does not match sum of migrationEdges operationCount (${totalEdgeOps})`, );(Both already use template literals; the fix is to ensure both placeholders are formatted identically if one changes in the future.)
🤖 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/3-targets/postgres/src/core/migrations/runner.ts` around lines 651 - 653, The error message in runner.ts mixes template literal interpolation for planOpsLength with string concatenation for totalEdgeOps; update the throw in the block that checks if (totalEdgeOps !== planOpsLength) so both values are injected consistently (e.g., use a single template literal) and keep the existing descriptive text unchanged, referencing the variables totalEdgeOps and planOpsLength in the message.
🤖 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/migrations/types.ts`:
- Around line 254-257: The operations array type widens promised entries to
Promise<MigrationPlanOperation>, allowing non-SQL shapes into
SqlMigrationPlan.operations; tighten the type by replacing
Promise<MigrationPlanOperation> with
Promise<SqlMigrationPlanOperation<TTargetDetails>> so the promised branch
preserves SQL-typed operations. Update the readonly operations declaration (the
union using SqlMigrationPlanOperation<TTargetDetails> and Promise<...>) to use
Promise<SqlMigrationPlanOperation<TTargetDetails>> and ensure any call sites
creating promised operations produce that SQL-typed promise.
In `@packages/3-targets/3-targets/sqlite/src/core/migrations/render-ops.ts`:
- Around line 13-19: Remove the redundant bare cast after blindCast in the array
mapping: the blindCast already ensures that the call to OpFactoryCall.toOp
returns Op | Promise<Op>, so drop the trailing "as Op | Promise<Op>" and return
the blindCast(c).toOp(lowerer) result directly; update the expression in the map
where blindCast<{ toOp(lowerer?: DdlDriverLowerer): Op | Promise<Op> },
...>(c).toOp(lowerer) is used (references: blindCast, Op, OpFactoryCall.toOp,
lowerer, DdlDriverLowerer) so the code relies only on the typed blindCast
instead of an extra bare cast.
In `@packages/3-targets/6-adapters/postgres/src/core/control-adapter.ts`:
- Around line 1282-1283: The number-to-string conversion currently
unconditionally stringifies numeric `wire` values, producing invalid SQL when
`wire` is NaN or ±Infinity; update the numeric guard in control-adapter.ts (the
branch handling `if (typeof wire === 'number')` and the `wire` bigint branch) to
first check Number.isFinite(wire) and, if not finite, throw a clear error
(including the offending value and context) instead of returning a string; keep
the bigint branch as-is (bigints are always finite) and ensure the thrown
message references `wire` and the surrounding DEFAULT/SQL emission context so
callers can debug.
---
Outside diff comments:
In
`@packages/3-targets/6-adapters/sqlite/test/migrations/render-typescript.roundtrip.test.ts`:
- Line 233: The test currently calls renderOps(calls) without the adapter and
without awaiting the async results; update the assignment to pass the adapter
and await all promises, i.e. replace the call to renderOps(calls) with await
Promise.all(renderOps(calls, testAdapter)) (so the variable expected is built
from the resolved results), ensuring you reference the existing renderOps
function, the calls test input and the testAdapter variable used earlier in the
file.
---
Nitpick comments:
In `@packages/2-sql/9-family/test/field-event-planner.test.ts`:
- Around line 155-157: The test is casting o.toOp() to MigrationPlanOperation
synchronously; instead await the async-capable toOp() and then read .id so the
assertion supports both sync and async ops — update places using (o.toOp() as
MigrationPlanOperation).id (e.g., in the ops mapping assertions) to first
materialize toOp() for each op (using Promise.all over ops.map(...) or an
equivalent async map) and then assert against the resulting .id values; target
the toOp() call and MigrationPlanOperation usage in these test assertions.
In `@packages/3-targets/3-targets/postgres/src/core/migrations/runner.ts`:
- Around line 651-653: The error message in runner.ts mixes template literal
interpolation for planOpsLength with string concatenation for totalEdgeOps;
update the throw in the block that checks if (totalEdgeOps !== planOpsLength) so
both values are injected consistently (e.g., use a single template literal) and
keep the existing descriptive text unchanged, referencing the variables
totalEdgeOps and planOpsLength in the message.
In `@packages/3-targets/3-targets/postgres/test/migrations/render-ops.test.ts`:
- Around line 32-36: The test suite only checks synchronous error throwing for
renderOps when an op targets the wrong engine; add a companion async test that
constructs an OpFactoryCall-like object (use makeCall('sqlite',
'table.users.create', 'createTable') as base), override its toOp to return a
Promise resolving to the wrong-target Op, invoke renderOps with that async call
and assert the Promise.all(renderOps([...])) rejects with the same regex used in
the sync test; reference renderOps, makeCall, OpFactoryCall and toOp when
locating where to add the new async test.
🪄 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: 78d5dce6-5fa8-4af1-91fc-b02e26abecc2
⛔ Files ignored due to path filters (14)
projects/migrate-marker-ledger-to-typed-query-ast-commands/plan.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/01-async-interface-plumbing.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/02-pg-consumer-rewire.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/03-sqlite-consumer-rewire.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/plan.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/spec.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/sqlite-create-table-adoption/dispatches/01-lowerer-plumbing.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/sqlite-create-table-adoption/dispatches/02-sqlite-renderer-substrate-fix.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/sqlite-create-table-adoption/dispatches/03-create-table-call-refactor.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/sqlite-create-table-adoption/dispatches/04-sqlite-migration-create-table-method.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/sqlite-create-table-adoption/dispatches/05-reviewer-round-2.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/sqlite-create-table-adoption/plan.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/sqlite-create-table-adoption/spec.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/trace.jsonlis excluded by!projects/**
📒 Files selected for processing (76)
packages/1-framework/1-core/framework-components/src/control/control-migration-types.tspackages/1-framework/3-tooling/cli/src/commands/migration-plan.tspackages/1-framework/3-tooling/cli/src/migration-cli.tspackages/1-framework/3-tooling/migration/src/aggregate/strategies/synth.tspackages/1-framework/3-tooling/migration/src/migration-base.tspackages/1-framework/3-tooling/migration/test/migration-base.test.tspackages/2-sql/4-lanes/relational-core/src/ast/types.tspackages/2-sql/9-family/src/core/control-adapter.tspackages/2-sql/9-family/src/core/migrations/types.tspackages/2-sql/9-family/src/core/sql-migration.tspackages/2-sql/9-family/src/exports/control-adapter.tspackages/2-sql/9-family/test/field-event-planner.test.tspackages/2-sql/9-family/test/migrations.types.test.tspackages/3-extensions/pgvector/test/migrations/planner.behavior.test.tspackages/3-extensions/pgvector/test/migrations/planner.contract-to-schema-ir.test.tspackages/3-extensions/pgvector/test/migrations/planner.storage-types.test.tspackages/3-extensions/sqlite/README.mdpackages/3-extensions/sqlite/test/migration/re-export.test.tspackages/3-mongo-target/1-mongo-target/test/mongo-planner.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-produced-postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/render-ops.tspackages/3-targets/3-targets/postgres/src/core/migrations/runner.tspackages/3-targets/3-targets/postgres/test/migrations/render-ops.test.tspackages/3-targets/3-targets/sqlite/src/core/control-target.tspackages/3-targets/3-targets/sqlite/src/core/errors.tspackages/3-targets/3-targets/sqlite/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/sqlite/src/core/migrations/planner-produced-sqlite-migration.tspackages/3-targets/3-targets/sqlite/src/core/migrations/planner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/render-ops.tspackages/3-targets/3-targets/sqlite/src/core/migrations/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/sqlite-migration.tspackages/3-targets/3-targets/sqlite/src/exports/migration.tspackages/3-targets/3-targets/sqlite/test/migrations/nullability-backfill.test.tspackages/3-targets/3-targets/sqlite/test/migrations/op-factory-call.test.tspackages/3-targets/3-targets/sqlite/test/migrations/planner.authoring-surface.test.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/test/lower-to-driver-statement.test.tspackages/3-targets/6-adapters/postgres/test/migrations/cross-namespace-fk.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/fixtures/runner-fixtures.tspackages/3-targets/6-adapters/postgres/test/migrations/op-factory-call.construction.test.tspackages/3-targets/6-adapters/postgres/test/migrations/op-factory-call.lowering.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.codec-field-event.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.cross-space-fk-ddl.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.fk-config.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.reconciliation.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.referential-actions.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.semantic-satisfaction.test.tspackages/3-targets/6-adapters/postgres/test/migrations/planner.storage-types.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/render-typescript.roundtrip.test.tspackages/3-targets/6-adapters/postgres/test/migrations/runner.unbound-namespace.integration.test.tspackages/3-targets/6-adapters/sqlite/package.jsonpackages/3-targets/6-adapters/sqlite/src/core/control-adapter.tspackages/3-targets/6-adapters/sqlite/src/core/ddl-renderer.tspackages/3-targets/6-adapters/sqlite/test/ddl-create-table-lowering.test.tspackages/3-targets/6-adapters/sqlite/test/ddl-table-constraints-lowering.test.tspackages/3-targets/6-adapters/sqlite/test/lower-to-driver-statement.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/create-table-call-byte-parity.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/fixtures/runner-fixtures.tspackages/3-targets/6-adapters/sqlite/test/migrations/planner-introspection.integration.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/planner.codec-field-event.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/planner.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/render-typescript.roundtrip.test.tspackages/3-targets/6-adapters/sqlite/tsconfig.jsonpackages/3-targets/6-adapters/sqlite/tsconfig.test.jsonskills/extension-author/prisma-next-extension-upgrade/upgrades/0.12-to-0.13/instructions.mdskills/upgrade/prisma-next-upgrade/upgrades/0.12-to-0.13/instructions.mdtest/e2e/framework/test/sqlite/migrations/harness.tstest/integration/test/cross-package/postgres-control-policy-planner.test.tstest/integration/test/cross-package/postgres-issue-planner.test.tstest/integration/test/mongo/migration-e2e.test.tstest/integration/test/referential-actions.integration.test.ts
|
Ack on the review-body findings (#794 (review)):
|
…R, walker unification The PR #794 three-pass review + maintainer review found the slice's named outcome unimplemented: lowerToDriverStatement type-branches on raw values; the codec is never consulted. Root cause: the DDL AST carries no codec identity (unlike the query AST, where ParamRef carries CodecRef) and toOp passes contract: {}. Spec + plan amended with the settled design: - Renames: DriverStatement → ExecutableStatement (executability is the property, "Driver" named the consumer); lowerToDriverStatement → lowerToExecutableStatement; DdlDriverLowerer → ExecutableStatementLowerer with SqlControlAdapter extends (dedupe). - SerializedQueryPlan subsumed by ExecutableStatement (single consumer is PG data-transform; the sql-shaped type never belonged in the framework layer). - Codec wiring: DdlColumn gains optional codecRef, planner populates from StorageColumn.codecId, walker resolves via codecLookup and awaits encode; contract-free authored calls fall back to the documented wire-scalar rule. contract: {} call sites die. - PlannerProduced*Migration.operations memoizes (review F1 — required before nondeterministic codecs land). - Marker/ledger bootstrap migrates off lower()-for-DDL; lower() then rejects DDL; old DDL renderer paths deleted from both adapters (resolves original AC9 decisively — one DDL walker per adapter). - Fixtures gain Date/bigint/JSON + extension-codec default coverage (the extension-codec case is what distinguishes codec routing from type-branching). Three new dispatches: D4 mechanical sweep (renames + quick wins), D5 codec wiring, D6 bootstrap migration + walker deletion. D4 brief authored. Review artifacts updated with maintainer-comment resolutions under reviews/pr-794/. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…zedQueryPlan subsumption + review quick wins Mechanical sweep from the PR #794 review round. No codec wiring (D5), no walker deletion (D6) — those are the next dispatches. Renames (executability is the property; "Driver" named the consumer): - DriverStatement -> ExecutableStatement (relational-core/ast) - lowerToDriverStatement -> lowerToExecutableStatement (both adapters) - DdlDriverLowerer -> ExecutableStatementLowerer; internal visitor/ helper/test-file names follow Interface fixes: - SqlControlAdapter extends ExecutableStatementLowerer; the duplicated method declaration is deleted (review M-Arch-2) - SerializedQueryPlan deleted from framework-components: its only consumer was PG data-transform.ts, and the {sql, params} shape never belonged in the framework layer -- it imports ExecutableStatement from relational-core now (review M-Arch-1, partial: type unification) Quick wins (review F5/F6/F7/F10/F12/F13): - isThenable helper in @prisma-next/utils/promise replaces instanceof Promise at the sql-migration + both render-ops sites (cross-realm robustness) - ifDefined for the conditional constraint spreads in both targets' CreateTableCall - delete the mechanical re-export parity tests (SQLite + PG twin -- TypeScript proves re-export parity at compile time) - SQLite renderOps gains the target-id assertion PG already had - sqliteInlineLiteral gains the non-finite-number guard PG had; both inline helpers guard invalid Date before toISOString(); PG Uint8Array inline literal casts ::nativeType instead of hardcoded ::bytea - byte-parity test renamed create-table-call-lowering (the old name overpromised -- it no longer compares against the pre-slice path) Gates run manually (pre-commit lint-deps-focused OOMs on 31 files): pnpm typecheck / test:packages / fixtures:check / lint:deps green; lint:casts delta 0. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e in walker, memoize The load-bearing dispatch: the DDL walker now actually calls the codec. Before this, lowerToExecutableStatement type-branched on the raw JS value and never consulted a codec — the slice's named outcome was unimplemented (PR #794 review finding). - DdlColumn gains codecRef?: CodecRef. The planner populates it from the resolved column's codecId (+ typeParams) at both targets' issue-planner construction sites; the SQLite inline-autoincrement path leaves it unset. - Both adapters' DDL walkers became plain async functions (replacing the sync visitor — the closed createTable/createSchema node set doesn't need the visitor): for a codec-bearing literal default they resolve codecLookup.get(codecRef.codecId) and await codec.encode(value, {}), then inline the wire result. Codec-less columns keep the documented RawSqlLiteral wire-scalar fallback. - contract: {} placeholders dropped from the DDL toOp sites; DDL lowering resolves codecs from the column, not the lowering context. - PlannerProduced{Postgres,Sqlite}Migration.operations memoized: one lowering per instance (review F1 — required before a nondeterministic codec could make displayOps and the executed SQL diverge). Proof of routing (both targets): a unit test registers a codec whose encode transforms its input ('plaintext' -> 'ENC:PLAINTEXT') and asserts the DDL contains the encoded form with the raw value absent — the difference between routing and type-branching. A second test pins the no-codec fallback. On the empirical "does a shipping codec diverge" question: builtin codecs (text/uuid/jsonb/timestamptz/int/...) encode to output byte-identical to the type-branching fallback, which is why the existing adapter tests still pass and no migration golden regenerated. Divergence only manifests with a transforming codec (encryption/custom domain), none of which ship today. The synthetic-codec unit test is therefore the correct proof — it exercises the transforming path that real future codecs will hit, without fabricating a shipping divergence that doesn't exist. Routing is correct-by-construction; its observable effect is latent until a transforming codec lands. isThenable (D4) rewritten cast-free via in-narrowing (keeps lint:casts delta 0). Gates run manually (pre-commit lint-deps-focused OOMs on large sets): pnpm typecheck (138/138) / test:packages / fixtures:check / lint:deps green; lint:casts delta 0. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lice Operator decision on the #768 review: the precheck/postcheck verification SELECTs (raw SQL in every *Call.toOp()) are the project's own "express, don't concatenate" violation, but converting them is deferred — not absorbed into the two large in-flight PRs (#768, #794). Carve it out of Phase 2 as an explicit named slice (typed-migration-verification-queries) and record the substrate finding: the contract-free builder's projection is column-only, so the slice must add aggregate (count) + comparison projection first (the AST already has AggregateExpr.count). Not a defense of the raw SQL — a scheduled removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/3-targets/3-targets/postgres/src/core/migrations/render-ops.ts (1)
20-25: ⚡ Quick winDrop the bare casts from the target-assertion helpers.
Line 21 in the Postgres helper and Line 14 in the SQLite helper both use
as Partial<Op>just to readtarget?.id. That reintroduces an unchecked production cast on the new hot path even though both files already haveblindCastavailable. Narrow throughblindCastorcastAson the minimal shape instead.As per coding guidelines, "No bare
asin production code. UseblindCast<T, "Reason">orcastAs<T>from@prisma-next/utils/casts."🤖 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/3-targets/postgres/src/core/migrations/render-ops.ts` around lines 20 - 25, The assertPostgresOp helper uses a bare cast "(op as Partial<Op>)" to access target?.id; replace that unchecked cast with the provided safe cast utilities (blindCast<T,"Reason"> or castAs<T>) from `@prisma-next/utils/casts` to narrow op to the minimal shape needed (e.g., an object with target?: { id?: string }). Update assertPostgresOp (and the analogous SQLite helper) to call blindCast or castAs on op before reading target?.id, keeping the same error message and preserving the asserts op is Op signature.Source: Coding guidelines
🤖 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/3-targets/sqlite/test/migrations/planner.authoring-surface.test.ts`:
- Around line 15-18: The stubLowerer currently implements lower() to return a
successful value which allows tests to pass via the deprecated sync path; update
stubLowerer so its lower() implementation throws (e.g., throw new Error('lower()
should not be used in these tests')) while leaving lowerToExecuteRequest() as
the async success path, ensuring the test suite enforces usage of
lowerToExecuteRequest().
In `@packages/3-targets/6-adapters/postgres/src/core/adapter.ts`:
- Around line 77-79: The error message in lower() references a non-existent API
lowerToExecuteRequest(); add a corresponding async method named
lowerToExecuteRequest to PostgresAdapterImpl and ensure createPostgresAdapter
returns it on the adapter surface. Implement lowerToExecuteRequest to perform
the async inline codec encoding for DDL default literals and return the
ExecuteRequest (mirroring the logic necessary when lowering DDL), and update any
exports/types so callers can call adapter.lowerToExecuteRequest().
In `@packages/3-targets/6-adapters/postgres/src/core/control-codecs.ts`:
- Around line 1-5: The import statement currently brings in AnyQueryAst,
ContractCodecRegistry, and LoweredStatement as value imports; update it to use
type-only imports by changing the import to use "import type" for those symbols
(AnyQueryAst, ContractCodecRegistry, LoweredStatement) so they are treated as
types only and satisfy the useImportType lint rule.
In `@packages/3-targets/6-adapters/sqlite/src/core/control-codecs.ts`:
- Around line 1-5: The imports in control-codecs.ts are currently runtime
imports but the symbols AnyQueryAst, ContractCodecRegistry, and LoweredStatement
are only used as types; change the import to use TypeScript's type-only form by
replacing the current import with an import type for these three identifiers
from '`@prisma-next/sql-relational-core/ast`' so they are erased at emit and
satisfy Biome linting (locate the import at the top of control-codecs.ts where
AnyQueryAst, ContractCodecRegistry, and LoweredStatement are listed).
---
Nitpick comments:
In `@packages/3-targets/3-targets/postgres/src/core/migrations/render-ops.ts`:
- Around line 20-25: The assertPostgresOp helper uses a bare cast "(op as
Partial<Op>)" to access target?.id; replace that unchecked cast with the
provided safe cast utilities (blindCast<T,"Reason"> or castAs<T>) from
`@prisma-next/utils/casts` to narrow op to the minimal shape needed (e.g., an
object with target?: { id?: string }). Update assertPostgresOp (and the
analogous SQLite helper) to call blindCast or castAs on op before reading
target?.id, keeping the same error message and preserving the asserts op is Op
signature.
🪄 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: 8a8be3e5-fc3e-480a-b451-1c0d8242c10a
⛔ Files ignored due to path filters (10)
projects/migrate-marker-ledger-to-typed-query-ast-commands/plan.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/04-mechanical-sweep.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/05-codec-wiring.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/06-bootstrap-migration-walker-deletion.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/07-executable-statement-convergence.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/08-control-query-param-encoding.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/dispatches/09-query-branch-end-to-end-test.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/plan.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/spec.mdis excluded by!projects/**projects/migrate-marker-ledger-to-typed-query-ast-commands/spec.mdis excluded by!projects/**
📒 Files selected for processing (59)
packages/1-framework/0-foundation/utils/package.jsonpackages/1-framework/0-foundation/utils/src/exports/promise.tspackages/1-framework/0-foundation/utils/src/promise.tspackages/1-framework/0-foundation/utils/test/promise.test.tspackages/1-framework/0-foundation/utils/tsdown.config.tspackages/1-framework/1-core/framework-components/src/control/control-migration-types.tspackages/1-framework/1-core/framework-components/src/exports/control.tspackages/2-sql/4-lanes/relational-core/src/ast/ddl-types.tspackages/2-sql/4-lanes/relational-core/src/ast/driver-types.tspackages/2-sql/4-lanes/relational-core/src/contract-free/column.tspackages/2-sql/9-family/src/core/control-adapter.tspackages/2-sql/9-family/src/core/control-instance.tspackages/2-sql/9-family/src/core/sql-migration.tspackages/2-sql/9-family/src/exports/control-adapter.tspackages/3-extensions/postgres/test/migration/re-export.test.tspackages/3-extensions/sqlite/test/migration/re-export.test.tspackages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/postgres/src/core/migrations/operations/data-transform.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner-produced-postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/planner.tspackages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.tspackages/3-targets/3-targets/postgres/src/core/migrations/render-ops.tspackages/3-targets/3-targets/postgres/src/core/migrations/runner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/issue-planner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.tspackages/3-targets/3-targets/sqlite/src/core/migrations/planner-ddl-builders.tspackages/3-targets/3-targets/sqlite/src/core/migrations/planner-produced-sqlite-migration.tspackages/3-targets/3-targets/sqlite/src/core/migrations/planner.tspackages/3-targets/3-targets/sqlite/src/core/migrations/render-ops.tspackages/3-targets/3-targets/sqlite/src/core/migrations/runner.tspackages/3-targets/3-targets/sqlite/test/migrations/nullability-backfill.test.tspackages/3-targets/3-targets/sqlite/test/migrations/op-factory-call.test.tspackages/3-targets/3-targets/sqlite/test/migrations/planner.authoring-surface.test.tspackages/3-targets/6-adapters/postgres/src/core/adapter.tspackages/3-targets/6-adapters/postgres/src/core/control-adapter.tspackages/3-targets/6-adapters/postgres/src/core/control-codecs.tspackages/3-targets/6-adapters/postgres/src/core/ddl-renderer.tspackages/3-targets/6-adapters/postgres/src/core/marker-ledger.tspackages/3-targets/6-adapters/postgres/test/ddl-create-table-lowering.test.tspackages/3-targets/6-adapters/postgres/test/ddl-table-constraints-lowering.test.tspackages/3-targets/6-adapters/postgres/test/lower-to-execute-request.test.tspackages/3-targets/6-adapters/postgres/test/migrations/data-transform.test.tspackages/3-targets/6-adapters/postgres/test/migrations/fixtures/runner-fixtures.tspackages/3-targets/6-adapters/postgres/test/migrations/marker-ledger-writes.integration.test.tspackages/3-targets/6-adapters/postgres/test/migrations/marker-table-ddl.test.tspackages/3-targets/6-adapters/sqlite/src/core/adapter.tspackages/3-targets/6-adapters/sqlite/src/core/control-adapter.tspackages/3-targets/6-adapters/sqlite/src/core/control-codecs.tspackages/3-targets/6-adapters/sqlite/src/core/ddl-renderer.tspackages/3-targets/6-adapters/sqlite/src/core/marker-ledger.tspackages/3-targets/6-adapters/sqlite/test/ddl-create-table-lowering.test.tspackages/3-targets/6-adapters/sqlite/test/ddl-table-constraints-lowering.test.tspackages/3-targets/6-adapters/sqlite/test/lower-to-execute-request.test.tspackages/3-targets/6-adapters/sqlite/test/marker-ledger-writes.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/create-table-call-lowering.test.tspackages/3-targets/6-adapters/sqlite/test/migrations/fixtures/runner-fixtures.tspackages/3-targets/6-adapters/sqlite/tsconfig.jsontest/integration/test/postgres-bootstrap.ts
💤 Files with no reviewable changes (7)
- packages/3-targets/6-adapters/postgres/src/core/ddl-renderer.ts
- packages/3-extensions/postgres/test/migration/re-export.test.ts
- packages/1-framework/1-core/framework-components/src/exports/control.ts
- packages/3-targets/6-adapters/sqlite/test/ddl-create-table-lowering.test.ts
- packages/3-targets/6-adapters/sqlite/src/core/ddl-renderer.ts
- packages/3-extensions/sqlite/test/migration/re-export.test.ts
- packages/1-framework/1-core/framework-components/src/control/control-migration-types.ts
✅ Files skipped from review due to trivial changes (6)
- packages/1-framework/0-foundation/utils/src/exports/promise.ts
- packages/2-sql/4-lanes/relational-core/src/ast/driver-types.ts
- packages/1-framework/0-foundation/utils/tsdown.config.ts
- packages/1-framework/0-foundation/utils/package.json
- packages/2-sql/9-family/src/exports/control-adapter.ts
- packages/3-targets/6-adapters/sqlite/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/2-sql/9-family/src/core/sql-migration.ts
- packages/3-targets/3-targets/sqlite/test/migrations/op-factory-call.test.ts
- packages/3-targets/3-targets/sqlite/test/migrations/nullability-backfill.test.ts
- packages/3-targets/3-targets/sqlite/src/core/migrations/issue-planner.ts
- packages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.ts
👮 Files not reviewed due to content moderation or server errors (7)
- packages/2-sql/4-lanes/relational-core/src/ast/ddl-types.ts
- packages/2-sql/9-family/src/core/control-adapter.ts
- packages/2-sql/9-family/src/core/control-instance.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/issue-planner.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/op-factory-call.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/operations/data-transform.ts
- packages/3-targets/3-targets/postgres/src/core/migrations/postgres-migration.ts
…R, walker unification The PR #794 three-pass review + maintainer review found the slice's named outcome unimplemented: lowerToDriverStatement type-branches on raw values; the codec is never consulted. Root cause: the DDL AST carries no codec identity (unlike the query AST, where ParamRef carries CodecRef) and toOp passes contract: {}. Spec + plan amended with the settled design: - Renames: DriverStatement → ExecutableStatement (executability is the property, "Driver" named the consumer); lowerToDriverStatement → lowerToExecutableStatement; DdlDriverLowerer → ExecutableStatementLowerer with SqlControlAdapter extends (dedupe). - SerializedQueryPlan subsumed by ExecutableStatement (single consumer is PG data-transform; the sql-shaped type never belonged in the framework layer). - Codec wiring: DdlColumn gains optional codecRef, planner populates from StorageColumn.codecId, walker resolves via codecLookup and awaits encode; contract-free authored calls fall back to the documented wire-scalar rule. contract: {} call sites die. - PlannerProduced*Migration.operations memoizes (review F1 — required before nondeterministic codecs land). - Marker/ledger bootstrap migrates off lower()-for-DDL; lower() then rejects DDL; old DDL renderer paths deleted from both adapters (resolves original AC9 decisively — one DDL walker per adapter). - Fixtures gain Date/bigint/JSON + extension-codec default coverage (the extension-codec case is what distinguishes codec routing from type-branching). Three new dispatches: D4 mechanical sweep (renames + quick wins), D5 codec wiring, D6 bootstrap migration + walker deletion. D4 brief authored. Review artifacts updated with maintainer-comment resolutions under reviews/pr-794/. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…zedQueryPlan subsumption + review quick wins Mechanical sweep from the PR #794 review round. No codec wiring (D5), no walker deletion (D6) — those are the next dispatches. Renames (executability is the property; "Driver" named the consumer): - DriverStatement -> ExecutableStatement (relational-core/ast) - lowerToDriverStatement -> lowerToExecutableStatement (both adapters) - DdlDriverLowerer -> ExecutableStatementLowerer; internal visitor/ helper/test-file names follow Interface fixes: - SqlControlAdapter extends ExecutableStatementLowerer; the duplicated method declaration is deleted (review M-Arch-2) - SerializedQueryPlan deleted from framework-components: its only consumer was PG data-transform.ts, and the {sql, params} shape never belonged in the framework layer -- it imports ExecutableStatement from relational-core now (review M-Arch-1, partial: type unification) Quick wins (review F5/F6/F7/F10/F12/F13): - isThenable helper in @prisma-next/utils/promise replaces instanceof Promise at the sql-migration + both render-ops sites (cross-realm robustness) - ifDefined for the conditional constraint spreads in both targets' CreateTableCall - delete the mechanical re-export parity tests (SQLite + PG twin -- TypeScript proves re-export parity at compile time) - SQLite renderOps gains the target-id assertion PG already had - sqliteInlineLiteral gains the non-finite-number guard PG had; both inline helpers guard invalid Date before toISOString(); PG Uint8Array inline literal casts ::nativeType instead of hardcoded ::bytea - byte-parity test renamed create-table-call-lowering (the old name overpromised -- it no longer compares against the pre-slice path) Gates run manually (pre-commit lint-deps-focused OOMs on 31 files): pnpm typecheck / test:packages / fixtures:check / lint:deps green; lint:casts delta 0. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e in walker, memoize The load-bearing dispatch: the DDL walker now actually calls the codec. Before this, lowerToExecutableStatement type-branched on the raw JS value and never consulted a codec — the slice's named outcome was unimplemented (PR #794 review finding). - DdlColumn gains codecRef?: CodecRef. The planner populates it from the resolved column's codecId (+ typeParams) at both targets' issue-planner construction sites; the SQLite inline-autoincrement path leaves it unset. - Both adapters' DDL walkers became plain async functions (replacing the sync visitor — the closed createTable/createSchema node set doesn't need the visitor): for a codec-bearing literal default they resolve codecLookup.get(codecRef.codecId) and await codec.encode(value, {}), then inline the wire result. Codec-less columns keep the documented RawSqlLiteral wire-scalar fallback. - contract: {} placeholders dropped from the DDL toOp sites; DDL lowering resolves codecs from the column, not the lowering context. - PlannerProduced{Postgres,Sqlite}Migration.operations memoized: one lowering per instance (review F1 — required before a nondeterministic codec could make displayOps and the executed SQL diverge). Proof of routing (both targets): a unit test registers a codec whose encode transforms its input ('plaintext' -> 'ENC:PLAINTEXT') and asserts the DDL contains the encoded form with the raw value absent — the difference between routing and type-branching. A second test pins the no-codec fallback. On the empirical "does a shipping codec diverge" question: builtin codecs (text/uuid/jsonb/timestamptz/int/...) encode to output byte-identical to the type-branching fallback, which is why the existing adapter tests still pass and no migration golden regenerated. Divergence only manifests with a transforming codec (encryption/custom domain), none of which ship today. The synthetic-codec unit test is therefore the correct proof — it exercises the transforming path that real future codecs will hit, without fabricating a shipping divergence that doesn't exist. Routing is correct-by-construction; its observable effect is latent until a transforming codec lands. isThenable (D4) rewritten cast-free via in-narrowing (keeps lint:casts delta 0). Gates run manually (pre-commit lint-deps-focused OOMs on large sets): pnpm typecheck (138/138) / test:packages / fixtures:check / lint:deps green; lint:casts delta 0. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lice Operator decision on the #768 review: the precheck/postcheck verification SELECTs (raw SQL in every *Call.toOp()) are the project's own "express, don't concatenate" violation, but converting them is deferred — not absorbed into the two large in-flight PRs (#768, #794). Carve it out of Phase 2 as an explicit named slice (typed-migration-verification-queries) and record the substrate finding: the contract-free builder's projection is column-only, so the slice must add aggregate (count) + comparison projection first (the AST already has AggregateExpr.count). Not a defense of the raw SQL — a scheduled removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…ution The control-adapter conflict resolution between #794's DDL helpers and main's enum helpers left extractQuotedLiterals without its closing brace. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
6868fbc to
6d116a2
Compare
@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 📦
|
…R, walker unification The PR #794 three-pass review + maintainer review found the slice's named outcome unimplemented: lowerToDriverStatement type-branches on raw values; the codec is never consulted. Root cause: the DDL AST carries no codec identity (unlike the query AST, where ParamRef carries CodecRef) and toOp passes contract: {}. Spec + plan amended with the settled design: - Renames: DriverStatement → ExecutableStatement (executability is the property, "Driver" named the consumer); lowerToDriverStatement → lowerToExecutableStatement; DdlDriverLowerer → ExecutableStatementLowerer with SqlControlAdapter extends (dedupe). - SerializedQueryPlan subsumed by ExecutableStatement (single consumer is PG data-transform; the sql-shaped type never belonged in the framework layer). - Codec wiring: DdlColumn gains optional codecRef, planner populates from StorageColumn.codecId, walker resolves via codecLookup and awaits encode; contract-free authored calls fall back to the documented wire-scalar rule. contract: {} call sites die. - PlannerProduced*Migration.operations memoizes (review F1 — required before nondeterministic codecs land). - Marker/ledger bootstrap migrates off lower()-for-DDL; lower() then rejects DDL; old DDL renderer paths deleted from both adapters (resolves original AC9 decisively — one DDL walker per adapter). - Fixtures gain Date/bigint/JSON + extension-codec default coverage (the extension-codec case is what distinguishes codec routing from type-branching). Three new dispatches: D4 mechanical sweep (renames + quick wins), D5 codec wiring, D6 bootstrap migration + walker deletion. D4 brief authored. Review artifacts updated with maintainer-comment resolutions under reviews/pr-794/. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…zedQueryPlan subsumption + review quick wins Mechanical sweep from the PR #794 review round. No codec wiring (D5), no walker deletion (D6) — those are the next dispatches. Renames (executability is the property; "Driver" named the consumer): - DriverStatement -> ExecutableStatement (relational-core/ast) - lowerToDriverStatement -> lowerToExecutableStatement (both adapters) - DdlDriverLowerer -> ExecutableStatementLowerer; internal visitor/ helper/test-file names follow Interface fixes: - SqlControlAdapter extends ExecutableStatementLowerer; the duplicated method declaration is deleted (review M-Arch-2) - SerializedQueryPlan deleted from framework-components: its only consumer was PG data-transform.ts, and the {sql, params} shape never belonged in the framework layer -- it imports ExecutableStatement from relational-core now (review M-Arch-1, partial: type unification) Quick wins (review F5/F6/F7/F10/F12/F13): - isThenable helper in @prisma-next/utils/promise replaces instanceof Promise at the sql-migration + both render-ops sites (cross-realm robustness) - ifDefined for the conditional constraint spreads in both targets' CreateTableCall - delete the mechanical re-export parity tests (SQLite + PG twin -- TypeScript proves re-export parity at compile time) - SQLite renderOps gains the target-id assertion PG already had - sqliteInlineLiteral gains the non-finite-number guard PG had; both inline helpers guard invalid Date before toISOString(); PG Uint8Array inline literal casts ::nativeType instead of hardcoded ::bytea - byte-parity test renamed create-table-call-lowering (the old name overpromised -- it no longer compares against the pre-slice path) Gates run manually (pre-commit lint-deps-focused OOMs on 31 files): pnpm typecheck / test:packages / fixtures:check / lint:deps green; lint:casts delta 0. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e in walker, memoize The load-bearing dispatch: the DDL walker now actually calls the codec. Before this, lowerToExecutableStatement type-branched on the raw JS value and never consulted a codec — the slice's named outcome was unimplemented (PR #794 review finding). - DdlColumn gains codecRef?: CodecRef. The planner populates it from the resolved column's codecId (+ typeParams) at both targets' issue-planner construction sites; the SQLite inline-autoincrement path leaves it unset. - Both adapters' DDL walkers became plain async functions (replacing the sync visitor — the closed createTable/createSchema node set doesn't need the visitor): for a codec-bearing literal default they resolve codecLookup.get(codecRef.codecId) and await codec.encode(value, {}), then inline the wire result. Codec-less columns keep the documented RawSqlLiteral wire-scalar fallback. - contract: {} placeholders dropped from the DDL toOp sites; DDL lowering resolves codecs from the column, not the lowering context. - PlannerProduced{Postgres,Sqlite}Migration.operations memoized: one lowering per instance (review F1 — required before a nondeterministic codec could make displayOps and the executed SQL diverge). Proof of routing (both targets): a unit test registers a codec whose encode transforms its input ('plaintext' -> 'ENC:PLAINTEXT') and asserts the DDL contains the encoded form with the raw value absent — the difference between routing and type-branching. A second test pins the no-codec fallback. On the empirical "does a shipping codec diverge" question: builtin codecs (text/uuid/jsonb/timestamptz/int/...) encode to output byte-identical to the type-branching fallback, which is why the existing adapter tests still pass and no migration golden regenerated. Divergence only manifests with a transforming codec (encryption/custom domain), none of which ship today. The synthetic-codec unit test is therefore the correct proof — it exercises the transforming path that real future codecs will hit, without fabricating a shipping divergence that doesn't exist. Routing is correct-by-construction; its observable effect is latent until a transforming codec lands. isThenable (D4) rewritten cast-free via in-narrowing (keeps lint:casts delta 0). Gates run manually (pre-commit lint-deps-focused OOMs on large sets): pnpm typecheck (138/138) / test:packages / fixtures:check / lint:deps green; lint:casts delta 0. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lice Operator decision on the #768 review: the precheck/postcheck verification SELECTs (raw SQL in every *Call.toOp()) are the project's own "express, don't concatenate" violation, but converting them is deferred — not absorbed into the two large in-flight PRs (#768, #794). Carve it out of Phase 2 as an explicit named slice (typed-migration-verification-queries) and record the substrate finding: the contract-free builder's projection is column-only, so the slice must add aggregate (count) + comparison projection first (the AST already has AggregateExpr.count). Not a defense of the raw SQL — a scheduled removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…ution The control-adapter conflict resolution between #794's DDL helpers and main's enum helpers left extractQuotedLiterals without its closing brace. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…R, walker unification The PR #794 three-pass review + maintainer review found the slice's named outcome unimplemented: lowerToDriverStatement type-branches on raw values; the codec is never consulted. Root cause: the DDL AST carries no codec identity (unlike the query AST, where ParamRef carries CodecRef) and toOp passes contract: {}. Spec + plan amended with the settled design: - Renames: DriverStatement → ExecutableStatement (executability is the property, "Driver" named the consumer); lowerToDriverStatement → lowerToExecutableStatement; DdlDriverLowerer → ExecutableStatementLowerer with SqlControlAdapter extends (dedupe). - SerializedQueryPlan subsumed by ExecutableStatement (single consumer is PG data-transform; the sql-shaped type never belonged in the framework layer). - Codec wiring: DdlColumn gains optional codecRef, planner populates from StorageColumn.codecId, walker resolves via codecLookup and awaits encode; contract-free authored calls fall back to the documented wire-scalar rule. contract: {} call sites die. - PlannerProduced*Migration.operations memoizes (review F1 — required before nondeterministic codecs land). - Marker/ledger bootstrap migrates off lower()-for-DDL; lower() then rejects DDL; old DDL renderer paths deleted from both adapters (resolves original AC9 decisively — one DDL walker per adapter). - Fixtures gain Date/bigint/JSON + extension-codec default coverage (the extension-codec case is what distinguishes codec routing from type-branching). Three new dispatches: D4 mechanical sweep (renames + quick wins), D5 codec wiring, D6 bootstrap migration + walker deletion. D4 brief authored. Review artifacts updated with maintainer-comment resolutions under reviews/pr-794/. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…zedQueryPlan subsumption + review quick wins Mechanical sweep from the PR #794 review round. No codec wiring (D5), no walker deletion (D6) — those are the next dispatches. Renames (executability is the property; "Driver" named the consumer): - DriverStatement -> ExecutableStatement (relational-core/ast) - lowerToDriverStatement -> lowerToExecutableStatement (both adapters) - DdlDriverLowerer -> ExecutableStatementLowerer; internal visitor/ helper/test-file names follow Interface fixes: - SqlControlAdapter extends ExecutableStatementLowerer; the duplicated method declaration is deleted (review M-Arch-2) - SerializedQueryPlan deleted from framework-components: its only consumer was PG data-transform.ts, and the {sql, params} shape never belonged in the framework layer -- it imports ExecutableStatement from relational-core now (review M-Arch-1, partial: type unification) Quick wins (review F5/F6/F7/F10/F12/F13): - isThenable helper in @prisma-next/utils/promise replaces instanceof Promise at the sql-migration + both render-ops sites (cross-realm robustness) - ifDefined for the conditional constraint spreads in both targets' CreateTableCall - delete the mechanical re-export parity tests (SQLite + PG twin -- TypeScript proves re-export parity at compile time) - SQLite renderOps gains the target-id assertion PG already had - sqliteInlineLiteral gains the non-finite-number guard PG had; both inline helpers guard invalid Date before toISOString(); PG Uint8Array inline literal casts ::nativeType instead of hardcoded ::bytea - byte-parity test renamed create-table-call-lowering (the old name overpromised -- it no longer compares against the pre-slice path) Gates run manually (pre-commit lint-deps-focused OOMs on 31 files): pnpm typecheck / test:packages / fixtures:check / lint:deps green; lint:casts delta 0. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…memoize, fixtures)
The load-bearing dispatch: makes the slice's name true. DdlColumn gains
codecRef; planner populates from StorageColumn.codecId; the DDL walkers
resolve via codecLookup.get(codecId) and await codec.encode before
inlining the wire result; contract-free authored calls fall back to the
documented wire-scalar rule. contract:{} placeholders removed.
operations getter memoized (review F1). Fixtures gain Date/bigint/JSON
+ one divergent extension-codec default (the case that distinguishes
routing from type-branching).
Brief names the two real design questions as halt conditions: the
async-encode-in-sync-visitor wrinkle, and the get(id)-vs-forCodecRef
resolution granularity for parameterized codecs.
Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e in walker, memoize The load-bearing dispatch: the DDL walker now actually calls the codec. Before this, lowerToExecutableStatement type-branched on the raw JS value and never consulted a codec — the slice's named outcome was unimplemented (PR #794 review finding). - DdlColumn gains codecRef?: CodecRef. The planner populates it from the resolved column's codecId (+ typeParams) at both targets' issue-planner construction sites; the SQLite inline-autoincrement path leaves it unset. - Both adapters' DDL walkers became plain async functions (replacing the sync visitor — the closed createTable/createSchema node set doesn't need the visitor): for a codec-bearing literal default they resolve codecLookup.get(codecRef.codecId) and await codec.encode(value, {}), then inline the wire result. Codec-less columns keep the documented RawSqlLiteral wire-scalar fallback. - contract: {} placeholders dropped from the DDL toOp sites; DDL lowering resolves codecs from the column, not the lowering context. - PlannerProduced{Postgres,Sqlite}Migration.operations memoized: one lowering per instance (review F1 — required before a nondeterministic codec could make displayOps and the executed SQL diverge). Proof of routing (both targets): a unit test registers a codec whose encode transforms its input ('plaintext' -> 'ENC:PLAINTEXT') and asserts the DDL contains the encoded form with the raw value absent — the difference between routing and type-branching. A second test pins the no-codec fallback. On the empirical "does a shipping codec diverge" question: builtin codecs (text/uuid/jsonb/timestamptz/int/...) encode to output byte-identical to the type-branching fallback, which is why the existing adapter tests still pass and no migration golden regenerated. Divergence only manifests with a transforming codec (encryption/custom domain), none of which ship today. The synthetic-codec unit test is therefore the correct proof — it exercises the transforming path that real future codecs will hit, without fabricating a shipping divergence that doesn't exist. Routing is correct-by-construction; its observable effect is latent until a transforming codec lands. isThenable (D4) rewritten cast-free via in-narrowing (keeps lint:casts delta 0). Gates run manually (pre-commit lint-deps-focused OOMs on large sets): pnpm typecheck (138/138) / test:packages / fixtures:check / lint:deps green; lint:casts delta 0. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ts DDL, delete old renderer The project's namesake deliverable: migrate the marker/ledger bootstrap DDL onto lowerToExecutableStatement (the typed-query-AST path the codec slice built). Driven by the structural truth that DDL-lowering is inherently async (DEFAULT clauses can't be parameterized, so the codec must run inline) — so lower() (sync) rejects DDL on every adapter, lowerToExecutableStatement becomes THE DDL path, and the old renderLoweredDdl renderer is then unused and deleted wholesale (both ddl-renderer.ts files are entirely private helpers serving it). Operator decisions captured: runtime adapter's lower() rejects DDL too (it doesn't need DDL today; add the async path when it does); touching the runtime adapter is fine. Brief names the one real risk as a halt: the two independently-written walkers could format bootstrap DDL differently — cosmetic drift regenerates goldens, a real format regression halts. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ejects DDL; delete old renderer
The parent project's namesake deliverable. DDL-lowering is inherently
async (DEFAULT clauses can't be parameterized, so the codec must run
inline), so:
- The marker/ledger bootstrap (control-instance.ts sign-marker loop +
lowerAst, which both runners drive) now lowers through
lowerToExecutableStatement instead of the sync lower().
- lowerAst becomes async, returns ExecutableStatement.
- lower() rejects DDL on all four adapters (2 control + 2 runtime) with a
message pointing at lowerToExecutableStatement. The query branch is
unchanged. (Operator decision: the runtime adapter doesn't need DDL
today; add the async path when it does.)
- The old DDL renderer (renderLoweredDdl + defaultVisitor + constraint/
column helpers) is deleted wholesale from both adapters' ddl-renderer.ts
— every function in those files served renderLoweredDdl, which now has
no caller. One DDL walker per adapter.
Regression fixed in passing (caught by the e2e suite): the SQLite
now() -> datetime('now') mapping. The contract canonicalizes
CURRENT_TIMESTAMP to now(); the old flat-spec renderer mapped it back
(planner-ddl-builders.ts:70) but the new walker (which SqliteCreateTable
moved onto in slice 5) emitted raw DEFAULT (now()), which real SQLite
rejects. The mapping now lives in the SQLite walker — the dialect
authority — pinned by a unit test and the additive/widening e2e.
Test migrations (all off lower()-for-DDL, onto lowerToExecutableStatement):
runner executeStatement typed to ExecutableStatement; both targets'
runner-fixtures bootstrap helpers; marker-ledger-writes (unit + PG
integration); marker-table-ddl; ddl-table-constraints-lowering (both
targets); ddl-create-table-lowering (PG, migrated; SQLite deleted as
redundant in D4); the integration postgres-bootstrap helper; plus the
dead "lower output unchanged after D1" premise tests deleted. Constraint
and cast SQL verified byte-identical between old renderer and new walker
(the migrated .toBe() assertions pass unchanged — no bootstrap drift,
confirmed by fixtures:check producing no regen).
Gates: typecheck 138/138; test:packages 9949; test:e2e 105;
test:integration 1065/1066 (the one failure is a pre-existing PG
"portal does not exist" concurrent-query flake — passes in isolation);
fixtures:check green (no drift); lint:deps green; lint:casts delta 0.
Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…qlExecuteRequest
ExecutableStatement was a redundant fourth name for {sql, params}. The driver
port already defines SqlExecuteRequest (relational-core/ast/driver-types) and
owns the shape at the correct layer. Delete ExecutableStatement and route every
reference — adapters, runners, op-factory steps, planner, tests — through
SqlExecuteRequest. Its params field is optional; step-construction sites default
to [] and execute sites guard on length.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Will Madden <madden@prisma.io>
lowerToExecuteRequest's query branch forwarded literal params raw, contradicting its driver-ready contract; the downstream control driver binds raw, so a non-wire-ready literal (Date/bigint/JSON/transforming codec) was mis-bound. Encode literal params via the shared control-query encoder (deriveParamMetadata + encodeParamsWithMetadata + CONTROL_CODECS), extracted from marker-ledger so both share one path. Route Postgres dataTransform — the one reachable caller that lowers real user query ASTs — through the encoded path instead of extracting raw slots. Bind slots in control queries now throw. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
The query-branch encode was only proven at the helper level. Add an end-to-end test per target that calls adapter.lowerToExecuteRequest on a query binding a literal whose builtin codec observably transforms the value, asserting the encoded wire value lands in params and the raw JS value does not. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Review nit: replace hand-rolled conditional spreads with ifDefined. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Project artifact omitted when the convergence code landed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…lice Operator decision on the #768 review: the precheck/postcheck verification SELECTs (raw SQL in every *Call.toOp()) are the project's own "express, don't concatenate" violation, but converting them is deferred — not absorbed into the two large in-flight PRs (#768, #794). Carve it out of Phase 2 as an explicit named slice (typed-migration-verification-queries) and record the substrate finding: the contract-free builder's projection is column-only, so the slice must add aggregate (count) + comparison projection first (the AST already has AggregateExpr.count). Not a defense of the raw SQL — a scheduled removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…ution The control-adapter conflict resolution between #794's DDL helpers and main's enum helpers left extractQuotedLiterals without its closing brace. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
DDL execute steps now include a params array; regenerate fixtures to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…on diffs DDL execute steps now carry params[], and pgvector tests await lazy plan operations; declare both as incidental substrate diffs so check:upgrade-coverage passes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…04c)
A01: make stubLowerer.lower() throw so the authoring-surface suite actually
guards the async lowerToExecuteRequest() path (mirrors production where
lower() rejects DDL).
A02: reword the DDL-rejection error in both runtime adapters (postgres +
sqlite) — the old message told callers to use lowerToExecuteRequest(), which
lives on the control adapter, not the runtime surface.
A04a/A04b: convert inline-type imports in postgres and sqlite control-codecs.ts
to statement-level `import type` to satisfy biome useImportType.
A04c: replace bare `(op as Partial<Op>)` in assertPostgresOp / assertSqliteOp
with blindCast narrowed to `{ target?: { id?: string } }` (no-bare-casts rule).
Cast count delta: -2.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Will Madden <madden@prisma.io>
72da8f0 to
4832dd3
Compare
- sqlite-migration.ts: restore newline before `>(stack.adapter.create(stack))` that was collapsed onto the string literal line (formatter error) - promise.test.ts: suppress noThenProperty on intentional thenable fixtures - lower-to-execute-request.test.ts (sqlite + postgres): swap `ast` import before `contract-free` import to match biome's required sort order - data-transform.test.ts (postgres): swap `contract-free` before `data-transform` import for the same reason Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
… query encoding (prisma#807) ## At a glance A migration backfills an encrypted column: ```ts dataTransform({ id: 'data_migration.backfill-ssn', table: 'user', run: () => db.user.update({ ssn: '123-45-6789' }).where(({ ssn }) => ssn.isNull()), }) ``` `ssn` uses a custom codec — say a CipherStash-style encrypting codec, or a parameterized one like `pg/vector@1`. When the runner lowers this step for execution, the literal `'123-45-6789'` has to be encoded by `ssn`'s codec before it reaches the driver. **Before:** it threw `RUNTIME.CODEC_DESCRIPTOR_MISSING`. The control path could only encode the *builtin* codecs (text, int, jsonb, …); it had no entry for a custom or parameterized one. **After:** the column's actual codec runs, and the encrypted wire value is what the driver receives. This is latent today — no custom codec ships yet — so nothing is broken in practice. The point is to unblock them: an encrypted column simply can't be the target of a migration backfill until this works. ## The decision The control adapter already receives a `CodecLookup` — the injected handle it uses to resolve a column's codec. Rather than introduce a second way to get codecs, we **expand that one interface** with a params-aware resolver: ```ts interface CodecLookup { get(id): Codec | undefined; // existing — a representative, empty-params instance getForRef(id, typeParams): Codec | undefined; // new — the real codec for this column's params // …targetTypesFor, metaFor, renderOutputTypeFor } ``` The query branch of `lowerToExecuteRequest` — the path a Postgres `dataTransform` takes — now resolves each parameter's codec through `getForRef` and encodes with it, instead of falling back to a builtin-only registry. ## Why the query branch was the gap `lowerToExecuteRequest` has two branches, and only one of them was already correct. - The **DDL branch** (column defaults in `CREATE TABLE`) resolves codecs through the injected `CodecLookup`, so a custom default already worked on Postgres. - The **query branch** (the `UPDATE … WHERE` a `dataTransform` lowers) did not. It encoded params through a `CONTROL_CODECS` registry built from the target's *builtin* codec list. A column whose codec wasn't builtin had no entry there, so encoding threw. So the fix is to make the query branch resolve codecs the same way the DDL branch already does — through the one injected lookup. ## Why `get` wasn't enough: parameterized codecs `CodecLookup` already has a `get(id)`, so the tempting shortcut is `forCodecRef: ref => codecLookup.get(ref.codecId)`. It doesn't hold up for the case that matters. `get` returns a *representative* codec built with empty params. That's fine for a codec whose behavior doesn't depend on its params, and wrong for one that does: `pg/vector@1` needs its `length`; a per-column encrypting codec needs its key reference. For those, `get` either hands back a misconfigured instance or — when the factory requires params — returns nothing at all. `getForRef(id, typeParams)` is the faithful version. It validates the column's `typeParams` against the codec's schema and calls the factory with them, producing the exact codec the column uses. And the descriptors it needs are already in hand: `extractCodecLookup` builds the `CodecLookup` *from* them, so `getForRef` just keeps them and resolves on demand. ## A second gap this closes: SQLite parity The SQLite control adapter was constructed with no codec lookup at all — builtin-only even for its DDL branch. Threading `stack.codecLookup` into its construction (needed anyway for the query branch) closes that too: SQLite DDL defaults now honor custom codecs the same way Postgres already did. ## Scope Follow-up to prisma#794, which routed DDL default literals through codecs. Expanding the interface ripples to every `CodecLookup` implementor: the two real ones resolve faithfully (the builtin lookups delegate to `extractCodecLookup`), and the inline stubs in tests gain a `getForRef: () => undefined`. `buildContractCodecRegistry` stays private; no new exports. ## Verification - `pnpm typecheck` — 138 tasks pass - `pnpm test:packages` — 801 files pass (the two flaky PGlite integration tests are unrelated) - `pnpm lint:packages` — 67 tasks pass - `pnpm lint:casts` — delta 0 - `pnpm fixtures:check` — clean - `pnpm lint:deps` — clean ## Alternatives considered - **Thread the descriptors as a separate channel.** The first cut added a `codecDescriptors` array to `ControlStack`, passed it as a second adapter constructor argument, and exported `buildContractCodecRegistry` for the adapters to call. Dropped: it adds a second injected input alongside `CodecLookup`, when `CodecLookup` is already built from those same descriptors. One handle, expanded, beats two carrying overlapping information. - **Bridge through `get`.** `forCodecRef: ref => codecLookup.get(ref.codecId)` — covered above. It drops params, so parameterized and key-bearing codecs resolve wrong or not at all, which is the exact failure this PR removes. - **Leave it builtin-only.** The query branch only ever sees builtin codecs today, so nothing breaks in practice. Rejected because this slice exists for custom codecs: shipping a control path that silently can't encode them — while the DDL branch can — bakes in an asymmetry the next person would have to rediscover. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added enhanced codec registry system supporting descriptor lookup and materialization with validation. * Improved extension codec handling with proper descriptor resolution and type validation. * **Bug Fixes** * Enhanced error handling for missing codec descriptors with descriptive runtime errors. * Added type parameter validation for parameterized codecs with synchronous validation enforcement. * **Refactoring** * Centralized runtime error handling abstraction. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Will Madden <madden@prisma.io> Co-authored-by: Will Madden <madden@prisma.io> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
At a glance
A Postgres
timestamptzcolumn with aDatedefault. Before, the migration planner fell throughdefaultVisitor.literal'sJSON.stringify(value)branch and emitted invalid SQL —DEFAULT '"2026-01-01T00:00:00.000Z"'::timestamptz(JSON quotes inside the literal); abigintdefault threw at plan time. After, the column's codec encodes the value and it's inlined correctly:That's the visible bug. The PR underneath it does two larger things: it makes the adapter genuinely route DDL default literals through the column's codec (not a hand-rolled
typeofladder), and it moves the marker/ledger bootstrap onto the same typed-AST lowering path — the parent project's namesake deliverable.The decision: a second lowering method, because DDL-lowering is async
The SQL family already had one adapter method,
lower(ast, ctx): LoweredStatement— sync. It's sync on purpose: the runtime query path produces a SQL template with$Nplaceholders and defers codec encoding to the driver (encodeParams), so middleware can mutate raw param values before they're encoded. That model can't lower a DDLDEFAULTclause: you can't writeDEFAULT $1, so the codec has to run inline, andcodec.encodeis async.So the adapter grows a second, independent method:
SqlExecuteRequestis{ sql, params? }whereparamsare codec-encoded, driver-ready wire values — the SQL analogue of Mongo'sMigrationStep.command(a wire-typed payload the driver executes directly). It is not a new type: the driver port already defined it (relational-core/ast/driver-types), at the correct low layer. This PR deletes the three redundant near-duplicates that had accreted around the same{ sql, params }shape —ExecutableStatement,DriverStatement,SerializedQueryPlan— and converges every consumer ontoSqlExecuteRequest. Itsparamsis optional (a param-less statement is valid); step-construction sites default to[]and execute sites guard on length.lower()now rejects DDL on every adapter (control and runtime) and points the caller atlowerToExecuteRequest. The runtime query path —lower(),LoweredStatement,encodeParams, the middleware lifecycle — is otherwise untouched.How codec routing actually works
DdlColumngains an optionalcodecRef, mirroring how the query AST'sParamRefalready carries one. The planner populates it from the column'scodecId(+typeParams). The walker resolves the codec via the adapter'scodecLookupandawaitscodec.encode(value), then inlines the wire result with dialect-correct quoting and (PG)::nativeTypecast. Contract-freecol()calls with no codec follow the documentedRawSqlLiteralwire-scalar fallback.The same encoding discipline now applies to the control path's query params, not just DDL defaults:
lowerToExecuteRequest's query branch codec-encodes its literal params (it previously forwarded raw values), so the method's "driver-ready params" contract holds for every input. The PostgresdataTransformmigration step — the one control path that lowers real user query ASTs whose literals can beDate/bigint/JSON — routes through the same encoded path instead of extracting raw slot values.Because
toOp()now does async work,*Call.toOp()returnsOp | Promise<Op>,MigrationPlan.operationswidens to(Op | Promise<Op>)[], and consumersawait Promise.all(plan.operations)once at the boundary. The user-authoring surface is unchanged —override get operations() { return [this.createTable(...)] }stays a plain sync array literal; the async-ness is invisible to the migration author (git diff main -- examples/**/migration.tsis empty).Marker/ledger on the typed path; one DDL walker per adapter
The marker/ledger bootstrap (the sign-marker loop +
lowerAst, driven by both runners) now lowers throughlowerToExecuteRequestinstead of the synclower(). With nothing left callinglower()for DDL, the oldddl-renderer.ts(renderLoweredDdl+defaultVisitor+ constraint/column helpers) is deleted wholesale from both adapters. Net −384 lines; one DDL walker, not two.A latent regression this surfaced — relevant to #768
The e2e suite caught
unknown function: now()on SQLite. The contract canonicalizesCURRENT_TIMESTAMP→now(); the old flat-spec renderer mapped it back todatetime('now'), but the new walker (whichSqliteCreateTablemoved onto in slice 5 / #768) emitted rawDEFAULT (now()), which real SQLite rejects. It's been latent since slice 5. Fixed here in the SQLite walker (the dialect authority), pinned by a unit test and the additive/widening e2e. Heads-up: #768 carries the same latent bug until this lands on top of it.Honest caveat: codec routing has no observable effect on shipping codecs yet
For every builtin codec (text, uuid, jsonb, timestamptz, int, …)
encodeproduces output byte-identical to the old type-branching — which is why no migration golden regenerated andfixtures:checkis clean. Routing only diverges for a transforming codec (encryption, a custom domain), none of which ship today. The proof that routing works is therefore a unit test with a synthetic transforming codec ('plaintext'→'ENC:PLAINTEXT', raw value absent from the SQL) on both targets. The behaviour is correct-by-construction; its payoff is prospective (it's what makes a CipherStash-style encrypted column default possible).Stacking
This branch is stacked on #768 (slice 5, still open) and targets
main, so the GitHub diff includes slice 5. The TML-2867 change itself is ~98 files since the slice-5 tip; review from commit330dccc82(TML-2867 D1) onward, or once #768 merges this will rebase to a clean diff.Verification
pnpm typecheckgreen ·pnpm test:packages9950 ·pnpm test:e2e105 ·pnpm test:integrationgreen (the one failure is a pre-existing PGportal "C_n" does not existconcurrent-query flake — passes in isolation) ·pnpm fixtures:checkgreen (no golden drift; constraint + cast SQL verified byte-identical between the deleted renderer and the new walker via the migrated.toBe()assertions) ·pnpm lint:depsgreen ·pnpm lint:castsdelta 0.Alternatives considered
lower()itself async / async-everywhere. Deferred, not rejected on the merits. The runtime middleware lifecycle is already async, so an asynclower()wouldn't break it — the middleware seam is a pipeline ordering (lower → beforeExecute → encodeParams → driver), not a sync/async boundary. Two reasons we used a second method instead: (1) blast radius —lower()is on the query hot path (everyruntime.execute), so making it async ripples through the whole execution path, well beyond this slice's scope; (2) the output shapes legitimately differ —lower()returns raw params on purpose, becausebeforeExecute's param mutator rewrites raw values beforeencodeParams(the CipherStash case: middleware turns plaintext into what gets encoded). A control-plane lowering has no middleware, so it encodes inline. The cleaner long-term shape is the runtime-symmetry follow-up (M-Arch-1in the review artifacts): keeplower → middleware → encode, but expose the post-encode runtime result asSqlExecuteRequesttoo, unifying the wire type across both planes. Worth doing; out of scope here.Op[]at planner-construction time sooperationsstays sync. Works for planner-produced migrations but not user-authored ones (override get operations()callsthis.createTable(...)lazily). Returning(Op | Promise<Op>)[]and awaiting at the boundary handles both uniformly.toOp(lowerer). Considered (maintainer raised it); rejected to keep*Callinstances executable independently of any Migration context — recorded as a positive constraint in the review artifacts.Refs: TML-2867. Builds on #768 (TML-2859, slice 5). Review artifacts (architect / principal-engineer / walkthrough passes + maintainer-comment resolutions) under
projects/migrate-marker-ledger-to-typed-query-ast-commands/slices/codec-routed-ddl-defaults/.Summary by CodeRabbit
Breaking Changes
New Features
Improvements