diff --git a/backend/src/db-drizzle/CONVENTIONS.md b/backend/src/db-drizzle/CONVENTIONS.md index ee444eb..5f0412f 100644 --- a/backend/src/db-drizzle/CONVENTIONS.md +++ b/backend/src/db-drizzle/CONVENTIONS.md @@ -21,6 +21,28 @@ Run it against a database with every migration applied, after writing a migratio `drizzleSchema.integration.test.ts` fails when the mirror and the database disagree, on tables or on columns. That test exists because the drift is silent and already happened: the mirror sat missing `item_drafts` and `upload_links` from the moment #222 landed until #217, because the spike had copied it into `src/` by hand and nobody had reason to look. A stale mirror is worse than none — Drizzle infers row types from it, so a converted query type-checks against a schema the database does not have and fails at run time on a column that does not exist. +## The rule that will bite you hardest: columns in a `sql` template are unqualified + +Drizzle renders a column reference inside a `sql` template **without its table**. + +```ts +// WRONG. Generates: (SELECT COUNT(*)::int FROM "items" WHERE "category_id" = "id") +// Postgres resolves both sides against items, so the subquery correlates with +// itself and returns a plausible wrong number. +sql`(SELECT COUNT(*)::int FROM ${items} WHERE ${items.categoryId} = ${categories.id})` + +// RIGHT. Literal text, which is honest here because the fragment binds no values. +sql`(SELECT COUNT(*)::int FROM items WHERE items.category_id = categories.id)` +``` + +This is worse than the array trap below, because the array trap produces invalid SQL and fails loudly. This produces **valid SQL and quietly wrong data** — it type-checks, reads correctly, and executes without error. It was found in #218 only because an integration test asserted the count was 2 and got 1. + +So: any converted query containing a correlated subquery or a self-join needs a test asserting **values**, not just a status code. Write that test before converting. + +## The other one: a driver error code moves + +Drizzle wraps driver errors. A Postgres SQLSTATE that sat on `err.code` sits on `err.cause.code` after conversion, so a `catch` keyed on it still compiles, never matches, and turns a handled 409 into a 500. `adminCategories.ts` has `isUniqueViolation`, which accepts both shapes; reuse that pattern. Revisit every SQLSTATE-keyed catch when converting a file. + ## The rule that will bite you: arrays In a Drizzle `sql` template, an array interpolates as a **placeholder list**, not as one array parameter. @@ -43,6 +65,8 @@ That makes the #202 invariant — only placeholder indices may be interpolated i ## Both drivers run at once +Column names differ, and the difference is load-bearing. The mirror is camelCase (`parentId`, `sortOrder`); these APIs answer in snake_case, which the admin frontend reads. So a select must map explicitly — `{ parent_id: categories.parentId }` — rather than selecting the table. Selecting the table directly changes the JSON contract silently, and no test asserting status codes notices. `adminCategories.ts` writes that mapping once as `CATEGORY_COLUMNS` and infers the row type from it, which is also how the hand-declared row interfaces are retired. + `db` and `pool` are exported from `src/db.ts` and share one pool, deliberately. Conversion is file by file across 187 sites, so most queries will be raw `pg` for a long time and the two must not open separate connection pools — a transaction on one would be invisible to the other, and the configured limits would silently double. ## Not settled