TML-2859: SQLite migration planner builds CREATE TABLE as typed DDL through the adapter#768
Merged
Merged
Conversation
…ice 5)
Slice 5 / Phase 1 of the marker/ledger-via-typed-query-AST project. The
first SQLite slice in the planner-adoption arc — proves the planner-adoption
pattern works for SQLite by mirroring slice 4 (TML-2754, Postgres).
- Spec: SQLite CreateTableCall.toOp() builds the slice-1 CreateTable DDL-AST
node via the contract-free createTable(...) constructor and lowers through
SqlControlAdapter.lower(), replacing renderCreateTableSql string
concatenation on the planner's live path. Substrate from slice 4 (the
constraint node, contract-free constructor, SQLite adapter
constraint-rendering with 4 byte-parity tests) is fully in place; this
slice does the SQLite adapter byte-parity reconciliation + planner-side
migration.
- Plan: three INVEST-shippable dispatches. D1 plumbs Lowerer through the
SQLite planner stack (mechanical fan-out, no behaviour change); D2
migrates SqliteCreateTableCall.toOp(lowerer) with a byte-parity test that
drives toOp(lowerer) end-to-end; D3 adds SqliteMigration.createTable({...})
authoring method and drops the free createTable re-export from
@prisma-next/sqlite/migration (mirroring slice 4's PG-facade change).
- Project plan.md: fills in slice 5's Linear reference (was TBD).
- Initial trace events (spec-authored, plan-authored).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Will Madden <madden@prisma.io>
…fan-out)
Add `import type { Lowerer }` to op-factory-call.ts, render-ops.ts, planner.ts,
and planner-produced-sqlite-migration.ts. Change the abstract
`SqliteOpFactoryCallNode.toOp(): Op` signature to `toOp(lowerer?: Lowerer): Op`
and update all nine concrete `*Call.toOp` signatures to match (each body gains
an ignored `_lowerer?` param — no body change). Update `renderOps` to accept
and forward `lowerer?: Lowerer` via the same `blindCast` pattern used on the
Postgres path. Add `readonly #lowerer: Lowerer` to `SqliteMigrationPlanner` and
`TypeScriptRenderableSqliteMigration`, threading the value from
`createSqliteMigrationPlanner(lowerer: Lowerer)` through both
`TypeScriptRenderableSqliteMigration` constructor call sites. Update both
`createPlanner(adapter)` sites in `control-target.ts` to pass `adapter`. Update
all ten test call sites of `createSqliteMigrationPlanner()` — target-sqlite tests
use an inline `stubLowerer: Lowerer`; adapter-sqlite tests use `createSqliteAdapter()`
or `new SqliteControlAdapter()` (already in scope). Zero bare
`createSqliteMigrationPlanner()` calls remain across the repo.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Will Madden <madden@prisma.io>
D2 of slice 5 halted on a falsified-assumption stop-condition: the spec assumed the SQLite adapter substrate was byte-parity-ready, but grounding showed three gaps — the SQLite adapter renderer doesn't quote identifiers (the planner's `renderCreateTableSql` does, so adapter-output and planner-output cannot be byte-identical), the adapter renderer's CREATE TABLE indentation differs from the planner's (4-space vs 2-space), and `SqliteCreateTableCall` holds pre-rendered `defaultSql` strings rather than structured `DdlColumn[]` the way PG's `PostgresCreateTableCall` does after slice 4. Operator decision: expand slice 5 to fix the SQLite adapter renderer + replicate slice 4's `*Call holds DdlColumn[]` refactor on the SQLite side. Plan goes from 3 dispatches to 4: D1 (lowerer plumbing — done in `de813cb18`) + D2 (substrate fix: renderer convention) + D3 (consumer refactor + byte-parity proof) + D4 (authoring API, unchanged from original D3). Captures the in-flight D1 dispatch brief at `slices/sqlite-create-table-adoption/dispatches/01-lowerer-plumbing.md` and a draft D2 brief at `slices/sqlite-create-table-adoption/dispatches/02-create-table-lower-migration.md` (written before the stop-condition surfaced; D2 will be re-briefed after this commit lands and the spec amendment takes effect). Trace events appended for `spec-amended` / `plan-amended` / `falsified-assumption`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
…able-adoption Signed-off-by: Will Madden <madden@prisma.io>
The SQLite adapter DDL renderer (`packages/3-targets/6-adapters/sqlite/src/core/ddl-renderer.ts`) was emitting unquoted identifiers and using 4-space/2-space indentation that diverged from both the Postgres adapter renderer and the SQLite planner's `renderCreateTableSql`. This change adds `quoteIdentifier` (from `@prisma-next/target-sqlite/sql-utils`) and `quoteQualifiedIdentifier` to the SQLite renderer, applies them to every identifier reference (table name, column names, constraint names, FK column lists, FK ref-table), and switches indentation to the 2-space/0-space-close form used by the Postgres renderer. Expected-SQL literals in `ddl-create-table-lowering.test.ts` and `ddl-table-constraints-lowering.test.ts` are updated to match; a new test in the constraints file pins mixed-case column names and SQL reserved-word table/constraint names to demonstrate the behaviour the bare-string form could not provide. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
… (byte-parity)
Mirror slice 4's PG refactor onto SQLite.
`SqliteCreateTableCall` field shape changes from `spec: SqliteTableSpec` (a
flat struct of pre-rendered SQL fragments) to `columns: readonly DdlColumn[]
+ constraints?: readonly DdlTableConstraint[]` (structured AST nodes).
Constructor takes the structured shape directly. `toOp(lowerer)` builds a
typed DDL node via the contract-free `createTable({...})` constructor and
lowers it through `lowerer.lower(...)` — replacing the call to the free
`createTable(tableName, spec)` Op-builder on this path. The free Op-builder
and its `renderCreateTableSql` helper stay in `operations/tables.ts` for
`recreateTable`, which is Phase 2.
The upstream construction site in `issue-planner.ts` now builds the
structured shape: `StorageColumn` → `DdlColumn` instances via a new
`sqliteDefaultToDdlColumnDefault` helper (analogous to PG's
`postgresDefaultToDdlColumnDefault`); table constraints map to
`PrimaryKeyConstraint` / `ForeignKeyConstraint` / `UniqueConstraint`.
`renderTypeScript()` emits `this.createTable({ table, columns: [col(...),
...], constraints: [...] })` — the user-facing authoring form that
`SqliteMigration.createTable` (landing in the next dispatch) will consume.
Byte-parity proof at `adapter-sqlite/test/migrations/create-table-call-byte-parity.test.ts`:
the new test drives `SqliteCreateTableCall.toOp(lowerer)` end-to-end and
asserts byte-identity vs `renderCreateTableSql(tableName, spec)` across
five representative shapes (simple, composite PK, FK with referential
actions, table-level unique, autoincrement-inline-PK).
Roundtrip-test scope:
`render-typescript.roundtrip.test.ts` dropped `CreateTableCall` from its
operation list. The roundtrip evaluates rendered TS by calling `toOp` on
each construction, and CreateTableCall now needs both a Lowerer AND the
`this.createTable` method on the base class to round-trip — the method
doesn't land until the next dispatch. The remaining operations in the
test (AddColumn, CreateIndex, RawSql, …) still round-trip cleanly because
their toOp doesn't depend on either. Restore CreateTableCall coverage
once the migration method is available.
Includes the dispatch briefs for D2 and D3 as project-context artefacts;
the stale D3 brief from before the spec amendment is removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Will Madden <madden@prisma.io>
…-export drop
Adds `SqliteMigration.createTable({ table, columns, constraints? })` as a protected
instance method, mirroring `PostgresMigration.createTable`. The method builds a
`CreateTableCall` and lowers it through a `SqlControlAdapter<'sqlite'>` stored in the
constructor (materialized from `stack?.adapter`). A companion error function
`errorSqliteMigrationStackMissing` surfaces the misuse when the adapter is absent.
Drops the free `createTable` re-export from the target migration facade
(`packages/3-targets/3-targets/sqlite/src/exports/migration.ts`) and the extension
facade (`packages/3-extensions/sqlite`). The function stays in `operations/tables.ts`
for internal use by `recreateTable`. Adds `col`, `fn`, `lit`, `primaryKey`, `foreignKey`,
and `unique` from `@prisma-next/sql-relational-core/contract-free` to both facades
so authored migration files can import all column-builder helpers from the single
`@prisma-next/sqlite/migration` entrypoint.
Restores `CreateTableCall` coverage in the SQLite adapter roundtrip test now that
the base class method exists. Updates the byte-parity test to use the DDL renderer
directly as the oracle (instead of the now-private free `createTable` op factory).
Records the breaking API change in the 0.12→0.13 upgrade instructions for both user
and extension-author skill packages.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Will Madden <madden@prisma.io>
… byte-parity oracle, autoincrement guard Fix four reviewer findings from the slice-5 DoD pass. The literal-default renderer now emits 0/1 for booleans, a single-quoted ISO string for Date, and a bare integer string for bigint (defensive), matching the pre-slice renderDefaultLiteral output exactly. The byte-parity test is rewritten with a genuine cross-implementation oracle: each case independently provides a SqliteTableSpec for the pre-slice createTable path and DdlColumn[] for the new path, covering all ColumnDefaultLiteralInputValue kinds plus function defaults. sqliteDefaultToDdlColumnDefault now short-circuits autoincrement() to undefined at the helper boundary (mirroring the Postgres analogue) instead of forwarding a FunctionColumnDefault the renderer silently discards. Both the tableToDdlParts and renderColumn autoincrement sites carry cross-linking comments describing the type-smuggling convention, referencing TML-2866 for the structural fix. A tsconfig.test.json with rootDir widened to the monorepo packages root is added so the cross-package direct-source import in the parity test typechecks cleanly without relaxing the production tsconfig's rootDir. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Trace records D5 (round-2 fixes) dispatch lifecycle: dispatch-start, round-start, brief-issued, round-end (satisfied), dispatch-end. D4 and D5 brief artifacts capture the dispatched scope so the slice's audit trail is complete. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 13 minutes and 44 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR refactors the SQLite migration authoring surface by moving ChangesSQLite Migration API Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
size-limit report 📦
|
@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: |
wmadden
requested changes
Jun 10, 2026
wmadden-electric
pushed a commit
that referenced
this pull request
Jun 10, 2026
…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>
…stive default switch #768 review fixes. Raw-SQL precheck (op-factory-call) deferred to the typed-migration-verification-queries follow-up slice, per owner decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
Keep all release-note entries from both sides of the conflict in skills/extension-author/prisma-next-extension-upgrade/upgrades/0.12-to-0.13/instructions.md: - HEAD contributed: sqlite-create-table-method - main contributed: regen-extension-contracts-strip-empty-type-params, thread-namespace-id-through-codec-ref-resolver-spi, storage-namespace-envelope-re-emit No entries were contradictory; all were appended entries from independent PRs. Signed-off-by: Will Madden <madden@prisma.io>
wmadden
approved these changes
Jun 10, 2026
Two new bare as casts were introduced by this branch and flagged by pnpm lint:casts (delta +2 vs merge base): 1. tableToDdlParts in issue-planner.ts: the parameter type was Readonly<Record<...>> but buildColumnTypeSql takes Record<...>. Fixed by dropping the Readonly wrapper — no cast needed. 2. sqlite-migration.ts: stack.adapter.create(stack) cast to SqlControlAdapter<'sqlite'>. Replaced with blindCast with an explicit reason string. Signed-off-by: Will Madden <madden@prisma.io>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/3-targets/6-adapters/sqlite/tsconfig.test.json (1)
8-8: 💤 Low valueConsider simplifying the include to only test files.
Since the main
tsconfig.jsonalready includes and typecheckssrc/**/*.ts, the test config only needs to covertest/**/*.ts. This would make the separation of concerns clearer:- "include": ["src/**/*.ts", "test/**/*.ts"], + "include": ["test/**/*.ts"],TypeScript will still resolve imports from
src/via project references; the explicitsrc/**/*.tsinclusion is redundant.🤖 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/tsconfig.test.json` at line 8, Update the tsconfig.test.json "include" to only reference test files by removing the redundant "src/**/*.ts" pattern; modify the include array so it contains just "test/**/*.ts" (adjusting the "include" key in tsconfig.test.json) so tests are typechecked separately while allowing the main tsconfig to continue covering src via project references.packages/3-targets/6-adapters/sqlite/tsconfig.json (1)
8-8: ⚡ Quick winClarify the single-file exclusion pattern.
Excluding one specific test file (
create-table-call-byte-parity.test.ts) from the main TypeScript config while keepingtest/**/*.tsin the include list is an unusual pattern. Given thattsconfig.test.jsonexists and typechecks all tests with a widerrootDir, consider whether:
- All test files should be excluded from the main config and typechecked only via
tsconfig.test.json, or- This specific test requires the wider
rootDirand the single-file exclusion is intentional (in which case, a comment explaining why would be helpful).The current setup works but may surprise future maintainers who expect either "all tests in main config" or "all tests in separate test config" rather than a hybrid approach.
🤖 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/tsconfig.json` at line 8, The tsconfig.json currently excludes only "test/migrations/create-table-call-byte-parity.test.ts", which is surprising given tests are otherwise included and there is a separate tsconfig.test.json; either broaden the exclusion to exclude all tests from the main config (e.g. add "test/**/*.ts" to the "exclude" array so test files are only typechecked via tsconfig.test.json) or keep the single-file exclusion but add a clear inline comment in the same tsconfig.json explaining that this specific test needs the wider rootDir from tsconfig.test.json and therefore must be excluded here; refer to the "exclude" property and the filename "test/migrations/create-table-call-byte-parity.test.ts" when making the change.
🤖 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.
Nitpick comments:
In `@packages/3-targets/6-adapters/sqlite/tsconfig.json`:
- Line 8: The tsconfig.json currently excludes only
"test/migrations/create-table-call-byte-parity.test.ts", which is surprising
given tests are otherwise included and there is a separate tsconfig.test.json;
either broaden the exclusion to exclude all tests from the main config (e.g. add
"test/**/*.ts" to the "exclude" array so test files are only typechecked via
tsconfig.test.json) or keep the single-file exclusion but add a clear inline
comment in the same tsconfig.json explaining that this specific test needs the
wider rootDir from tsconfig.test.json and therefore must be excluded here; refer
to the "exclude" property and the filename
"test/migrations/create-table-call-byte-parity.test.ts" when making the change.
In `@packages/3-targets/6-adapters/sqlite/tsconfig.test.json`:
- Line 8: Update the tsconfig.test.json "include" to only reference test files
by removing the redundant "src/**/*.ts" pattern; modify the include array so it
contains just "test/**/*.ts" (adjusting the "include" key in tsconfig.test.json)
so tests are typechecked separately while allowing the main tsconfig to continue
covering src via project references.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 0c417730-57ea-4656-bbf5-f863a01c91e0
⛔ 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/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/dispatches/06-review-address.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 (27)
packages/3-extensions/sqlite/README.mdpackages/3-extensions/sqlite/test/migration/re-export.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/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/sqlite/package.jsonpackages/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/migrations/create-table-call-byte-parity.test.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.md
💤 Files with no reviewable changes (1)
- packages/3-extensions/sqlite/test/migration/re-export.test.ts
wmadden-electric
pushed a commit
that referenced
this pull request
Jun 10, 2026
New slice extracted from TML-2859 (slice 5) review finding F1. The DDL default-literal renderer's type-branching on the JS value is a parallel mini-codec; the runtime parameter-binding path already routes through Codec.encode(value, ctx). The slice unifies the two by routing DDL default literals through the same codec, making the chain async end-to-end. Spec covers: codec field on LiteralColumnDefault, async propagation through Lowerer + DDL render + *Call.toOp + MigrationPlanWithAuthoring Surface.operations getter, wireToDefaultLiteral helper per target, roll-back of TML-2859 D5's transitional type-branching. 3 dispatches: D1 async interface plumbing (no behaviour change) → D2 PG renderer codec routing → D3 SQLite renderer codec routing + D5 roll-back. Builds on TML-2859 (in PR #768) — needs slice 5 to land before merging. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wmadden-electric
pushed a commit
that referenced
this pull request
Jun 10, 2026
…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>
…ental extension diff Fixes #768 CI: the SQLite ddl-renderer's function-default handler emitted raw DEFAULT (now()), which SQLite rejects (e2e widening now() round-trip); add the datetime('now') mapping its siblings already have. Also declare the incidental 3-extensions diff (deleted re-export test) for check:upgrade-coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Will Madden <madden@prisma.io>
wmadden-electric
pushed a commit
that referenced
this pull request
Jun 10, 2026
New slice extracted from TML-2859 (slice 5) review finding F1. The DDL default-literal renderer's type-branching on the JS value is a parallel mini-codec; the runtime parameter-binding path already routes through Codec.encode(value, ctx). The slice unifies the two by routing DDL default literals through the same codec, making the chain async end-to-end. Spec covers: codec field on LiteralColumnDefault, async propagation through Lowerer + DDL render + *Call.toOp + MigrationPlanWithAuthoring Surface.operations getter, wireToDefaultLiteral helper per target, roll-back of TML-2859 D5's transitional type-branching. 3 dispatches: D1 async interface plumbing (no behaviour change) → D2 PG renderer codec routing → D3 SQLite renderer codec routing + D5 roll-back. Builds on TML-2859 (in PR #768) — needs slice 5 to land before merging. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wmadden-electric
pushed a commit
that referenced
this pull request
Jun 10, 2026
…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>
wmadden-electric
pushed a commit
that referenced
this pull request
Jun 10, 2026
New slice extracted from TML-2859 (slice 5) review finding F1. The DDL default-literal renderer's type-branching on the JS value is a parallel mini-codec; the runtime parameter-binding path already routes through Codec.encode(value, ctx). The slice unifies the two by routing DDL default literals through the same codec, making the chain async end-to-end. Spec covers: codec field on LiteralColumnDefault, async propagation through Lowerer + DDL render + *Call.toOp + MigrationPlanWithAuthoring Surface.operations getter, wireToDefaultLiteral helper per target, roll-back of TML-2859 D5's transitional type-branching. 3 dispatches: D1 async interface plumbing (no behaviour change) → D2 PG renderer codec routing → D3 SQLite renderer codec routing + D5 roll-back. Builds on TML-2859 (in PR #768) — needs slice 5 to land before merging. Signed-off-by: Will Madden <madden@prisma.io> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wmadden-electric
pushed a commit
that referenced
this pull request
Jun 10, 2026
…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>
paulwer
pushed a commit
to paulwer/prisma-next
that referenced
this pull request
Jun 11, 2026
…ped path (prisma#794) ## At a glance A Postgres `timestamptz` column with a `Date` default. **Before**, the migration planner fell through `defaultVisitor.literal`'s `JSON.stringify(value)` branch and emitted invalid SQL — `DEFAULT '"2026-01-01T00:00:00.000Z"'::timestamptz` (JSON quotes inside the literal); a `bigint` default threw at plan time. **After**, the column's codec encodes the value and it's inlined correctly: ```sql CREATE TABLE "config" ( "expires_at" timestamptz NOT NULL DEFAULT '2026-01-01T00:00:00.000Z'::timestamptz ); ``` 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 `typeof` ladder), 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 `$N` placeholders 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 DDL `DEFAULT` clause: you can't write `DEFAULT $1`, so the codec has to run **inline**, and `codec.encode` is async. So the adapter grows a second, independent method: ```ts interface SqlControlAdapter<T> extends ExecuteRequestLowerer { lower(ast, ctx): LoweredStatement; // existing — sync, query path, raw params lowerToExecuteRequest(ast, ctx): Promise<SqlExecuteRequest>; // new — async, fully encoded } ``` `SqlExecuteRequest` is `{ sql, params? }` where `params` are codec-encoded, driver-ready wire values — the SQL analogue of Mongo's `MigrationStep.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 onto `SqlExecuteRequest`. Its `params` is 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 at `lowerToExecuteRequest`. The runtime query path — `lower()`, `LoweredStatement`, `encodeParams`, the middleware lifecycle — is otherwise untouched. ## How codec routing actually works `DdlColumn` gains an optional `codecRef`, mirroring how the query AST's `ParamRef` already carries one. The planner populates it from the column's `codecId` (+ `typeParams`). The walker resolves the codec via the adapter's `codecLookup` and `await`s `codec.encode(value)`, then inlines the wire result with dialect-correct quoting and (PG) `::nativeType` cast. Contract-free `col()` calls with no codec follow the documented `RawSqlLiteral` wire-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 Postgres `dataTransform` migration step — the one control path that lowers real user query ASTs whose literals can be `Date`/`bigint`/JSON — routes through the same encoded path instead of extracting raw slot values. Because `toOp()` now does async work, `*Call.toOp()` returns `Op | Promise<Op>`, `MigrationPlan.operations` widens to `(Op | Promise<Op>)[]`, and consumers `await 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.ts` is 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 through `lowerToExecuteRequest` instead of the sync `lower()`. With nothing left calling `lower()` for DDL, the old `ddl-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 prisma#768 The e2e suite caught `unknown function: now()` on SQLite. The contract canonicalizes `CURRENT_TIMESTAMP` → `now()`; the old flat-spec renderer mapped it back to `datetime('now')`, but the new walker (which `SqliteCreateTable` moved onto **in slice 5 / prisma#768**) emitted raw `DEFAULT (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: prisma#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, …) `encode` produces output byte-identical to the old type-branching — which is why no migration golden regenerated and `fixtures:check` is 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 prisma#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 commit `330dccc82` (`TML-2867 D1`) onward, or once prisma#768 merges this will rebase to a clean diff. ## Verification `pnpm typecheck` green · `pnpm test:packages` 9950 · `pnpm test:e2e` 105 · `pnpm test:integration` green (the one failure is a pre-existing PG `portal "C_n" does not exist` concurrent-query flake — passes in isolation) · `pnpm fixtures:check` green (no golden drift; constraint + cast SQL verified byte-identical between the deleted renderer and the new walker via the migrated `.toBe()` assertions) · `pnpm lint:deps` green · `pnpm lint:casts` delta 0. ## Alternatives considered - **Make `lower()` itself async / async-everywhere.** *Deferred, not rejected on the merits.* The runtime middleware lifecycle is already async, so an async `lower()` 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 (every `runtime.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, because `beforeExecute`'s param mutator rewrites raw values *before* `encodeParams` (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-1` in the review artifacts): keep `lower → middleware → encode`, but expose the *post-encode* runtime result as `SqlExecuteRequest` too, unifying the wire type across both planes. Worth doing; out of scope here. - **Eager-materialize `Op[]` at planner-construction time so `operations` stays sync.** Works for planner-produced migrations but not user-authored ones (`override get operations()` calls `this.createTable(...)` lazily). Returning `(Op | Promise<Op>)[]` and awaiting at the boundary handles both uniformly. - **Access the lowerer via the Migration's stack instead of `toOp(lowerer)`.** Considered (maintainer raised it); rejected to keep `*Call` instances executable independently of any Migration context — recorded as a positive constraint in the review artifacts. Refs: TML-2867. Builds on prisma#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/`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Breaking Changes** * SQLite migrations: createTable is now a protected Migration method — use this.createTable(...) inside get operations(). * Migration operations may be produced asynchronously; callers/tooling should await resolved operations. * **New Features** * Migration helpers (col, lit, fn, primaryKey, foreignKey, unique) exported from the SQLite migration entry. * Adapters now provide async lowering into driver-ready execute requests. * **Improvements** * Execute steps include explicit params arrays; planners, runners, and tests updated for async resolution and codec-aware encoding. <!-- 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>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
At a glance
The SQLite migration planner's
CREATE TABLEno longer concatenates raw SQL — it builds a typed DDL-AST node and lowers it through the adapter at plan time, the same shape Postgres adopted in #751.The user-facing authoring form on the SQLite side becomes
this.createTable({ ... })onSqliteMigration— same shape Postgres already has.What changed
SqliteCreateTableCallrefactored (packages/3-targets/3-targets/sqlite/src/core/migrations/op-factory-call.ts). The class held a flatSqliteTableSpec(stringdefaultSqlfragments); now it holdsreadonly DdlColumn[]+readonly DdlTableConstraint[]?, mirroringPostgresCreateTableCall.toOp(lowerer)builds the contract-freecreateTable({...})node and lowers vialowerer.lower(node, { contract: {} }). The planner's upstream construction site atissue-planner.tsbuilds the structured node list from the same source data, with a newsqliteDefaultToDdlColumnDefaulthelper mirroring the Postgres analogue (autoincrement()short-circuits toundefined; literal defaults wrap inLiteralColumnDefault; expression defaults wrap inFunctionColumnDefault).SqliteMigrationauthoring API. Aprotected createTable({ table, columns, constraints? })method on the base class instantiates the call and lowers via the heldcontrolAdapter. The freecreateTable(tableName, spec)re-export is dropped from@prisma-next/sqlite/migration; the structural constructorscol,lit,fn,primaryKey,foreignKey,uniqueare re-exported instead. The user-facing breaking change is recorded via the upgrade-instructions skill.packages/3-targets/6-adapters/sqlite/src/core/ddl-renderer.ts). The SQLite DDL renderer didn't quote identifiers and used a different indent than the planner's canonical format. Fixed to quote identifiers and use the 2-space indent the planner emits, so the lowered SQL is byte-identical to the pre-slice output.quoteIdentifier/quoteQualifiedIdentifierhelpers added.The substrate-fix decision — adapter renderer convention
The first dispatch attempted the call-class refactor on top of the existing SQLite renderer. The byte-parity test failed immediately: the renderer didn't quote identifiers, used the wrong indent, and pre-rendered literal defaults inline rather than as a structured node. Three options surfaced:
Picked (1) — the planner's format is the canonical one (Postgres lowered to it in slice 4) and the renderer was the outlier. The slice's spec + plan amended to add the substrate fix as dispatch 2, the call-class refactor as dispatch 3, and the authoring API as dispatch 4.
Byte-parity — no behaviour change
A cross-implementation byte-parity test (
packages/3-targets/6-adapters/sqlite/test/migrations/create-table-call-byte-parity.test.ts) drivesSqliteCreateTableCall.toOp(lowerer)end-to-end with a realSqliteControlAdapter()and compares the lowered SQL to the pre-slicerenderCreateTableSqloutput for 12 representative shapes (simple, composite PK, FK with actions, table-level unique, autoincrement, plus 7 literal-default kinds). The oracle importscreateTabledirectly from the internaloperations/tables.tsmodule — the freecreateTableandrenderCreateTableSqlstay on disk becauserecreateTable(Phase 2) still calls them; only the facade re-export was dropped.The renderer's
defaultVisitor.literalwas expanded in D5 to type-branch correctly onboolean(→0/1),Date(→ ISO single-quoted), andbigint(→String(value)) for parity withrenderDefaultLiteral. This expansion is a transitional state that TML-2867 deletes by routing the value through the column's codec — the type-branching becomes dead once the codec produces the canonical wire form.Scope — deliberately SQLite
CreateTableonlyOther SQLite ops (
AddColumn/DropColumn/CreateIndex/DropIndex/DropTable/RecreateTable) stay on the existing string-build path. They're tracked as Phase 2 of the parent project (projects/migrate-marker-ledger-to-typed-query-ast-commands/).recreateTablestill calls the freecreateTable+renderCreateTableSqlfromoperations/tables.ts— preserved deliberately.A handful of inline-autoincrement smuggling sites in the planner (
tableToDdlPartsencodesPRIMARY KEY AUTOINCREMENTintoDdlColumn.type, the renderer substring-detects to special-case it) are cross-linked with comments naming the convention; the structural fix is tracked as TML-2866.Verification
git grep "createTable(this.tableName, this.spec)"→ zero matches. Byte-parity test passes for all 12 shapes.renderTypeScript()roundtrip test restored (executes the rendered TS scaffold throughtsxand assertsops.jsonequalsrenderOps(calls)).target-sqlite112 /adapter-sqlite194 /sqlitefacade 46 pass.pnpm fixtures:checkgreen;pnpm lint:depsgreen.Alternatives considered
codec.encode(value)calls directly. Extracted to TML-2867 because the change is cross-target (PG already shipped with the same type-branching shape) and async-propagation touches the frameworkMigrationPlanWithAuthoringSurfaceinterface — own slice's worth of work that deserves its own framing.projects/.../README.md) lands the planner-adoption pattern across all three targets (PG/SQLite/Mongo) before fanning out to per-target op vocabulary. Slice 5 is the SQLite pioneer.Refs: TML-2859. Related follow-ups: TML-2860 (slice-4 adapter-sqlite test debt), TML-2862 (pre-commit check for transient project refs), TML-2866 (DdlColumn.type smuggling structural fix), TML-2867 (codec-routed DDL default literals across PG + SQLite).
Summary by CodeRabbit
Breaking Changes
createTablemoved from exported function to protected method onMigrationbase class; call asthis.createTable(...)withinget operations().col,lit,fn,primaryKey,foreignKey,unique) now exported directly from@prisma-next/sqlite/migration.Improvements
Documentation