spike: evaluate Drizzle for type-safe queries and generated migrations #216

Closed
opened 2026-08-28 14:38:19 -05:00 by bermudalamb · 2 comments
Owner

Time-boxed spike, not a migration. The question is whether Drizzle earns a move away from hand-written pg queries and hand-written node-pg-migrate files, answered by converting the hardest thing in the codebase rather than the easiest.

What we have now

Raw pg (^8.12.0) with parameterised queries, and node-pg-migrate (^7.6.1) with six hand-written migrations. 187 pool.query/client.query call sites, concentrated in a handful of files:

File Call sites
routes/customers.ts 43
routes/cartCheckout.ts 30
routes/admin.ts 24
routes/cart.ts 19
routes/adminCustomers.ts 19
routes/shippingAddresses.ts 18
routes/adminCategories.ts 9
everything else ~25

Sixteen tables: admin_settings, cart_items, carts, categories, checkout_items, checkouts, customer_sessions, customer_tokens, customers, favorites, item_images, item_tags, items, orders, shipping_addresses, tags.

Nothing here is broken. Row shapes are hand-declared as interfaces (AdminItemRow, PublicItemRow, ItemRecord) and nothing enforces that they match the columns actually selected — a rename in a migration and a stale interface simply disagree, silently, until something reads undefined. That is the gap worth pricing.

Why Drizzle rather than the alternatives

Checked rather than recalled, on 2026-08-28:

  • Drizzle — Apache-2.0, actively maintained, ORM and query builder, Postgres with the pg driver, drizzle-kit for migrations. Keeps the SQL shape rather than hiding it.
  • Kysely — MIT, actively maintained, but explicitly "a type-safe and autocompletion-friendly TypeScript SQL query builder", not an ORM, and no schema-diff migration generation. It solves the typing half and leaves migrations exactly where they are.
  • Prisma — Apache-2.0, excellent migrations, but models live in a separate .prisma DSL rather than TypeScript, and its query API is not SQL-shaped. A bigger conceptual break for a codebase whose queries are deliberately readable as SQL.

Drizzle is the only one that answers both halves.

What the spike must produce

  1. drizzle-kit pull --init against the dev database. This introspects the live schema, generates schema.ts, and marks it as an applied baseline so later migrations diff against it. The six existing migrations stay as history and we stop adding to them. If this step does not cleanly reproduce all sixteen tables — including the recursive category tree and the item_tags join — that is most of the answer already.

  2. Convert buildItemFilterSql and its two callers (routes/items.ts, routes/admin.ts). Deliberately the hardest thing in the codebase: six optional clauses composed at run time, a recursive CTE for the category subtree, an ANY($n::int[]) tag match with a count equality, and array parameters. If the abstraction cannot express this cleanly it cannot express the rest, and picking an easy CRUD route first would prove nothing.

  3. Generate one real migration — a small additive column is enough — and judge the output against the standard the existing files set.

  4. Convert one high-density file end to end, routes/adminCategories.ts (9 sites, includes the SUBTREE_CTE recursive query), to get a real per-site cost rather than an extrapolation from one function.

Questions it has to answer

  • Does the injection invariant become structural? #202 documents that only placeholder indices may be interpolated into a clause, guarded by a comment and two mutation-tested unit tests. If a builder makes values bindable-only, that class of bug stops existing rather than being watched for, and #180's three S2077 hotspots stop being a thing anyone has to review. That is the strongest argument for doing this at all — confirm it is real and not just moved.
  • Does CommonJS work? backend/tsconfig.json is "module": "commonjs", "target": "ES2020". Drizzle is ESM-first. Confirm it works under the current build without forcing an ESM migration, because that would be a second large change wearing this one's clothes.
  • Does strict + noUncheckedIndexedAccess hold? Both are on. Inferred row types must survive them without spraying non-null assertions, or the type safety is nominal.
  • What happens to the hand-declared row interfaces? If AdminItemRow and friends can be derived from the schema instead of maintained beside it, that is the drift this is meant to close. If they have to stay, say so.
  • Cost: added dependency weight, build time, and a defensible per-site estimate for the remaining ~180.

Known costs, so they are priced rather than discovered

Generated migrations do not carry reasoning, and ours mostly are reasoning. 1787400000000_split-customer-name.js is about twenty lines of prose explaining that splitting on the first space is right for "Thom Lamb" and wrong for "Mary Jane Smith", why both columns are nullable although registration requires them, and why leaving them empty was rejected. drizzle-kit generate emits bare SQL. Either we commit to hand-editing generated migrations — keeping the writing and automating only the SQL, which is fine but is not "smoother" — or the repo quietly loses the habit that makes its history worth reading.

Data migrations cannot be generated at all. The name-splitting backfill is not a schema diff. Anything touching existing rows stays hand-written whatever the tooling, so the automation applies to a smaller share of our migrations than the headline suggests.

A second schema source of truth. schema.ts and the database can diverge the way sonar-project.properties already documents for sonar.projectVersion and the two package.json versions. Drizzle detects it on the next generate, which is better than the current situation, but it is a new coupling and should be named as one.

Non-goals

Not converting all 187 sites. Not touching the frontend. Not adopting Drizzle's relational query API — the SQL-like builder is the part being evaluated. Not removing node-pg-migrate in this issue; the existing migrations stay as applied history regardless of the outcome.

Done when

The spike branch shows the four conversions above with the suite green, and this issue carries a recommendation with numbers — adopt, adopt for new code only, or drop — so the decision on the remaining ~180 sites is made against evidence.

Refs

Follows a conversation about whether linq.ts could be used for queries. It cannot: it operates only on in-memory arrays, and no library can do true LINQ-to-SQL in TypeScript, because that depends on C# expression trees the provider reads rather than executes. Fluent type-safe builders are the available equivalent, which is how this landed on Drizzle.

Related: #202 (the injection invariant a builder could make structural), #180 (the S2077 hotspots it would retire).

Time-boxed spike, not a migration. The question is whether Drizzle earns a move away from hand-written `pg` queries and hand-written `node-pg-migrate` files, answered by converting the hardest thing in the codebase rather than the easiest. ## What we have now Raw `pg` (`^8.12.0`) with parameterised queries, and `node-pg-migrate` (`^7.6.1`) with six hand-written migrations. 187 `pool.query`/`client.query` call sites, concentrated in a handful of files: | File | Call sites | |---|---| | `routes/customers.ts` | 43 | | `routes/cartCheckout.ts` | 30 | | `routes/admin.ts` | 24 | | `routes/cart.ts` | 19 | | `routes/adminCustomers.ts` | 19 | | `routes/shippingAddresses.ts` | 18 | | `routes/adminCategories.ts` | 9 | | everything else | ~25 | Sixteen tables: `admin_settings`, `cart_items`, `carts`, `categories`, `checkout_items`, `checkouts`, `customer_sessions`, `customer_tokens`, `customers`, `favorites`, `item_images`, `item_tags`, `items`, `orders`, `shipping_addresses`, `tags`. Nothing here is broken. Row shapes are hand-declared as interfaces (`AdminItemRow`, `PublicItemRow`, `ItemRecord`) and nothing enforces that they match the columns actually selected — a rename in a migration and a stale interface simply disagree, silently, until something reads `undefined`. That is the gap worth pricing. ## Why Drizzle rather than the alternatives Checked rather than recalled, on 2026-08-28: - **Drizzle** — Apache-2.0, actively maintained, ORM *and* query builder, Postgres with the `pg` driver, `drizzle-kit` for migrations. Keeps the SQL shape rather than hiding it. - **Kysely** — MIT, actively maintained, but explicitly "a type-safe and autocompletion-friendly TypeScript SQL query builder", **not an ORM**, and no schema-diff migration generation. It solves the typing half and leaves migrations exactly where they are. - **Prisma** — Apache-2.0, excellent migrations, but models live in a separate `.prisma` DSL rather than TypeScript, and its query API is not SQL-shaped. A bigger conceptual break for a codebase whose queries are deliberately readable as SQL. Drizzle is the only one that answers both halves. ## What the spike must produce 1. **`drizzle-kit pull --init` against the dev database.** This introspects the live schema, generates `schema.ts`, and marks it as an applied baseline so later migrations diff against it. The six existing migrations stay as history and we stop adding to them. If this step does not cleanly reproduce all sixteen tables — including the recursive category tree and the `item_tags` join — that is most of the answer already. 2. **Convert `buildItemFilterSql` and its two callers** (`routes/items.ts`, `routes/admin.ts`). Deliberately the hardest thing in the codebase: six optional clauses composed at run time, a recursive CTE for the category subtree, an `ANY($n::int[])` tag match with a count equality, and array parameters. If the abstraction cannot express this cleanly it cannot express the rest, and picking an easy CRUD route first would prove nothing. 3. **Generate one real migration** — a small additive column is enough — and judge the output against the standard the existing files set. 4. **Convert one high-density file end to end**, `routes/adminCategories.ts` (9 sites, includes the `SUBTREE_CTE` recursive query), to get a real per-site cost rather than an extrapolation from one function. ## Questions it has to answer - **Does the injection invariant become structural?** #202 documents that only placeholder indices may be interpolated into a clause, guarded by a comment and two mutation-tested unit tests. If a builder makes values bindable-only, that class of bug stops existing rather than being watched for, and #180's three S2077 hotspots stop being a thing anyone has to review. That is the strongest argument for doing this at all — confirm it is real and not just moved. - **Does CommonJS work?** `backend/tsconfig.json` is `"module": "commonjs"`, `"target": "ES2020"`. Drizzle is ESM-first. Confirm it works under the current build without forcing an ESM migration, because that would be a second large change wearing this one's clothes. - **Does `strict` + `noUncheckedIndexedAccess` hold?** Both are on. Inferred row types must survive them without spraying non-null assertions, or the type safety is nominal. - **What happens to the hand-declared row interfaces?** If `AdminItemRow` and friends can be derived from the schema instead of maintained beside it, that is the drift this is meant to close. If they have to stay, say so. - **Cost:** added dependency weight, build time, and a defensible per-site estimate for the remaining ~180. ## Known costs, so they are priced rather than discovered **Generated migrations do not carry reasoning, and ours mostly are reasoning.** `1787400000000_split-customer-name.js` is about twenty lines of prose explaining that splitting on the first space is right for "Thom Lamb" and wrong for "Mary Jane Smith", why both columns are nullable although registration requires them, and why leaving them empty was rejected. `drizzle-kit generate` emits bare SQL. Either we commit to hand-editing generated migrations — keeping the writing and automating only the SQL, which is fine but is not "smoother" — or the repo quietly loses the habit that makes its history worth reading. **Data migrations cannot be generated at all.** The name-splitting backfill is not a schema diff. Anything touching existing rows stays hand-written whatever the tooling, so the automation applies to a smaller share of our migrations than the headline suggests. **A second schema source of truth.** `schema.ts` and the database can diverge the way `sonar-project.properties` already documents for `sonar.projectVersion` and the two `package.json` versions. Drizzle detects it on the next `generate`, which is better than the current situation, but it is a new coupling and should be named as one. ## Non-goals Not converting all 187 sites. Not touching the frontend. Not adopting Drizzle's relational query API — the SQL-like builder is the part being evaluated. Not removing `node-pg-migrate` in this issue; the existing migrations stay as applied history regardless of the outcome. ## Done when The spike branch shows the four conversions above with the suite green, and this issue carries a recommendation with numbers — adopt, adopt for new code only, or drop — so the decision on the remaining ~180 sites is made against evidence. ## Refs Follows a conversation about whether `linq.ts` could be used for queries. It cannot: it operates only on in-memory arrays, and no library can do true LINQ-to-SQL in TypeScript, because that depends on C# expression trees the provider reads rather than executes. Fluent type-safe builders are the available equivalent, which is how this landed on Drizzle. Related: #202 (the injection invariant a builder could make structural), #180 (the S2077 hotspots it would retire).
Author
Owner

Spike results — recommend adopting Drizzle, rejecting Tinqer

Branch feature/216-drizzle-spike, commit eff11c6. Both libraries were pointed at the same target: buildItemFilterSql, six clauses composed at run time, a recursive CTE for the category subtree, an ANY(...::int[]) tag match with a count equality. Nothing in src/routes or src/itemFilters.ts was changed.

Scope note up front: steps 1-3 were completed. Step 4, converting routes/adminCategories.ts end to end, was not done — Tinqer was added to the comparison partway through and took its budget. The per-site estimate below is therefore an extrapolation from one function, and is labelled as such rather than measured.

The CommonJS blocker is clear

The issue called this the likeliest early failure. backend/tsconfig.json is module: commonjs, target: ES2020, and Drizzle is ESM-first — but it compiles under the existing config and require()s at run time. No ESM migration is hiding inside this one.

Introspection is better than expected

drizzle-kit pull fetched 17 tables (the 16 real ones plus pgmigrations, excludable via tablesFilter), 104 columns, 8 indexes, 20 foreign keys. It got the parts I expected it to fumble:

  • the self-referencing categories.parent_id foreign key
  • both partial unique indexes, including the expressions and predicates: uniqueIndex("categories_child_name_uniq").using("btree", sqlparent_id, sqllower(name)).where(sql(parent_id IS NOT NULL))

The converted filter is equivalent, verified against the database

Five filter combinations, run against the dev database, comparing id lists from the current implementation and the Drizzle one:

Case Existing Drizzle
status only 1805 1805
price range 2145 2145
category subtree (recursive CTE) 4 4
status + price 1918 1918
nothing set 2145 2145

All five match exactly. The recursive CTE stays a sql template rather than $with(), because it sits inside an IN (...) subquery.

The injection question: yes, and stronger than I expected

In a Drizzle sql template, ${value} emits a bind parameter, not text. There is no way to spell "interpolate this value as SQL" by accident — the escape hatch that looks like a plain template literal does not behave like one. Passing "1); DROP TABLE items; --" as a status value:

hostile string present in SQL TEXT?  false
hostile string present in PARAMS?    true

That is the #202 invariant enforced by the type system instead of by a comment and two mutation tests, and it retires #180's three S2077 hotspots rather than leaving them reviewed-and-watched. This is the strongest argument for adopting, and it is real rather than relocated.

Two Drizzle findings to price in

Arrays do not bind the way the driver does. ${filters.categoryIds} expands into a placeholder list, producing ANY(($1, $2)::int[]) — which type-checks, reads correctly, and is invalid Postgres. It failed at run time, not at compile time. sql.param() is required and nothing warns you. Across 187 sites this is the kind of thing that passes review and fails in production; it should be a lint rule or a documented convention before any bulk conversion.

The first generated migration carried spurious index churn. Adding one column also emitted drops and recreations of the three expression indexes. Re-running with no schema change reports "nothing to migrate", so it settles rather than recurring — but that first migration needs hand-editing, and on a large table those recreations are real locks.

Both are manageable. Neither was visible from the documentation.

Tinqer: genuinely LINQ, and it cannot express this query

Worth saying plainly: I was wrong earlier to claim LINQ-to-SQL is impossible in TypeScript because it needs C# expression trees. Tinqer parses the lambda with OXC at run time and builds a real expression tree. The correct claim was that it is possible and rare.

But it cannot do what we need:

Shape Result
compound && condition works
array membership (p.statuses.includes(i.status)) works
ternary — clause optional at run time Failed to parse query
block body with if — clause optional at run time Failed to parse query

Those two failures are the only ways to make a clause conditional inside the lambda, and there is no raw-SQL escape hatch in its API. Six independent optional clauses would mean 64 hand-written plans, or neutral sentinels — which exist for price but not for "no category filter" versus a subtree walk, and not for the tag count.

The failure mode compounds it: defineSelect parses eagerly and throws, so an unsupported query type-checks cleanly and crashes when the module is first required. It is a runtime load-time error, not a type error. src/db-tinqer/probe.ts wraps every case in a function for exactly that reason.

Also material: version 0.0.27, 24 GitHub stars, and Postgres support via a pg-promise adapter rather than the pg driver already in use — a driver swap, not an addition.

MIT and interesting, and I would look again once it is past 1.0 and has a raw-SQL escape hatch. Not for 187 sites in a production shop today.

Recommendation

Adopt Drizzle, incrementally, new code first. It answers both halves of the original ask, the conversion is faithful on the hardest query we have, and it converts a documented security invariant into a structural one.

Suggested sequence, each its own issue:

  1. Land the schema and config, add tablesFilter to exclude pgmigrations, and write down the sql.param() array rule before anyone hits it.
  2. Convert routes/adminCategories.ts end to end (9 sites, includes the SUBTREE_CTE) to get the per-site cost actually measured rather than extrapolated. Rough guess from this spike is 15-30 minutes per site for the awkward ones and much less for simple CRUD, but that number should not be trusted until step 2 exists.
  3. Decide on node-pg-migrate separately. Nothing here forces that choice, and the reasoning-in-migrations cost recorded in the issue body still stands — generated migrations carry no prose, and data migrations cannot be generated at all.

What should not happen is a big-bang conversion of all 187 sites. The array-binding trap alone argues for converting a file at a time behind a green integration suite.

Verified: backend build clean, 280 unit tests and 255 integration tests pass, unchanged by the spike branch.

## Spike results — recommend adopting Drizzle, rejecting Tinqer Branch `feature/216-drizzle-spike`, commit `eff11c6`. Both libraries were pointed at the same target: `buildItemFilterSql`, six clauses composed at run time, a recursive CTE for the category subtree, an `ANY(...::int[])` tag match with a count equality. Nothing in `src/routes` or `src/itemFilters.ts` was changed. Scope note up front: steps 1-3 were completed. Step 4, converting `routes/adminCategories.ts` end to end, was **not** done — Tinqer was added to the comparison partway through and took its budget. The per-site estimate below is therefore an extrapolation from one function, and is labelled as such rather than measured. ### The CommonJS blocker is clear The issue called this the likeliest early failure. `backend/tsconfig.json` is `module: commonjs`, `target: ES2020`, and Drizzle is ESM-first — but it compiles under the existing config and `require()`s at run time. No ESM migration is hiding inside this one. ### Introspection is better than expected `drizzle-kit pull` fetched 17 tables (the 16 real ones plus `pgmigrations`, excludable via `tablesFilter`), 104 columns, 8 indexes, 20 foreign keys. It got the parts I expected it to fumble: - the self-referencing `categories.parent_id` foreign key - both partial unique indexes, including the expressions and predicates: `uniqueIndex("categories_child_name_uniq").using("btree", sql`parent_id`, sql`lower(name)`).where(sql`(parent_id IS NOT NULL)`)` ### The converted filter is equivalent, verified against the database Five filter combinations, run against the dev database, comparing id lists from the current implementation and the Drizzle one: | Case | Existing | Drizzle | |---|---|---| | status only | 1805 | 1805 | | price range | 2145 | 2145 | | category subtree (recursive CTE) | 4 | 4 | | status + price | 1918 | 1918 | | nothing set | 2145 | 2145 | All five match exactly. The recursive CTE stays a `sql` template rather than `$with()`, because it sits inside an `IN (...)` subquery. ### The injection question: yes, and stronger than I expected In a Drizzle `sql` template, `${value}` emits a **bind parameter**, not text. There is no way to spell "interpolate this value as SQL" by accident — the escape hatch that looks like a plain template literal does not behave like one. Passing `"1); DROP TABLE items; --"` as a status value: ``` hostile string present in SQL TEXT? false hostile string present in PARAMS? true ``` That is the #202 invariant enforced by the type system instead of by a comment and two mutation tests, and it retires #180's three S2077 hotspots rather than leaving them reviewed-and-watched. **This is the strongest argument for adopting, and it is real rather than relocated.** ### Two Drizzle findings to price in **Arrays do not bind the way the driver does.** `${filters.categoryIds}` expands into a placeholder list, producing `ANY(($1, $2)::int[])` — which type-checks, reads correctly, and is invalid Postgres. It failed at run time, not at compile time. `sql.param()` is required and nothing warns you. Across 187 sites this is the kind of thing that passes review and fails in production; it should be a lint rule or a documented convention before any bulk conversion. **The first generated migration carried spurious index churn.** Adding one column also emitted drops and recreations of the three expression indexes. Re-running with no schema change reports "nothing to migrate", so it settles rather than recurring — but that first migration needs hand-editing, and on a large table those recreations are real locks. Both are manageable. Neither was visible from the documentation. ### Tinqer: genuinely LINQ, and it cannot express this query Worth saying plainly: **I was wrong earlier** to claim LINQ-to-SQL is impossible in TypeScript because it needs C# expression trees. Tinqer parses the lambda with OXC at run time and builds a real expression tree. The correct claim was that it is possible and rare. But it cannot do what we need: | Shape | Result | |---|---| | compound `&&` condition | works | | array membership (`p.statuses.includes(i.status)`) | works | | ternary — clause optional at run time | **Failed to parse query** | | block body with `if` — clause optional at run time | **Failed to parse query** | Those two failures are the only ways to make a clause conditional inside the lambda, and there is no raw-SQL escape hatch in its API. Six independent optional clauses would mean 64 hand-written plans, or neutral sentinels — which exist for price but not for "no category filter" versus a subtree walk, and not for the tag count. The failure mode compounds it: `defineSelect` parses eagerly and **throws**, so an unsupported query type-checks cleanly and crashes when the module is first required. It is a runtime load-time error, not a type error. `src/db-tinqer/probe.ts` wraps every case in a function for exactly that reason. Also material: version `0.0.27`, 24 GitHub stars, and Postgres support via a `pg-promise` adapter rather than the `pg` driver already in use — a driver swap, not an addition. MIT and interesting, and I would look again once it is past 1.0 and has a raw-SQL escape hatch. Not for 187 sites in a production shop today. ## Recommendation **Adopt Drizzle, incrementally, new code first.** It answers both halves of the original ask, the conversion is faithful on the hardest query we have, and it converts a documented security invariant into a structural one. Suggested sequence, each its own issue: 1. Land the schema and config, add `tablesFilter` to exclude `pgmigrations`, and write down the `sql.param()` array rule before anyone hits it. 2. Convert `routes/adminCategories.ts` end to end (9 sites, includes the `SUBTREE_CTE`) to get the per-site cost actually measured rather than extrapolated. Rough guess from this spike is 15-30 minutes per site for the awkward ones and much less for simple CRUD, but that number should not be trusted until step 2 exists. 3. Decide on `node-pg-migrate` separately. Nothing here forces that choice, and the reasoning-in-migrations cost recorded in the issue body still stands — generated migrations carry no prose, and data migrations cannot be generated at all. What should **not** happen is a big-bang conversion of all 187 sites. The array-binding trap alone argues for converting a file at a time behind a green integration suite. Verified: backend build clean, 280 unit tests and 255 integration tests pass, unchanged by the spike branch.
Author
Owner

Decided: adopt Drizzle, incrementally, rejecting Tinqer.

Closing this. The spike answered what it was for — CommonJS works, introspection is faithful, the converted filter is byte-equivalent on the hardest query we have, and the injection invariant becomes structural rather than documented.

Step 4 of the original scope (convert routes/adminCategories.ts end to end) was not completed, so the per-site cost in the results comment is an extrapolation rather than a measurement. That step is not being quietly dropped — it is the whole point of #218.

Follow-on work:

  • #217 — land the schema, config and conventions, including the sql.param() array rule
  • #218 — convert routes/adminCategories.ts and measure the real per-site cost (absorbs step 4)
  • #219 — decide separately whether generated migrations replace node-pg-migrate

The spike branch feature/216-drizzle-spike (eff11c6) stays until #217 has taken what it needs from it, then it can go. It carries the throwaway src/db-tinqer/ probe and an experimental condition_note column that must not reach main.

Decided: **adopt Drizzle**, incrementally, rejecting Tinqer. Closing this. The spike answered what it was for — CommonJS works, introspection is faithful, the converted filter is byte-equivalent on the hardest query we have, and the injection invariant becomes structural rather than documented. Step 4 of the original scope (convert `routes/adminCategories.ts` end to end) was **not** completed, so the per-site cost in the results comment is an extrapolation rather than a measurement. That step is not being quietly dropped — it is the whole point of #218. Follow-on work: - #217 — land the schema, config and conventions, including the `sql.param()` array rule - #218 — convert `routes/adminCategories.ts` and measure the real per-site cost (absorbs step 4) - #219 — decide separately whether generated migrations replace `node-pg-migrate` The spike branch `feature/216-drizzle-spike` (`eff11c6`) stays until #217 has taken what it needs from it, then it can go. It carries the throwaway `src/db-tinqer/` probe and an experimental `condition_note` column that must **not** reach `main`.
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#216