ReviewOS

stacks/bun-query-builder

SQLite: a belongsTo FK added to an existing table is dropped — ADD COLUMN omits inline REFERENCES

#1154
Closed glennmichael123 opened this 8 days ago ·
8 days ago

Summary

On the SQLite dialect, adding a belongsTo foreign key to an already-existing table generates a bare ALTER TABLE … ADD COLUMN with no inline REFERENCES — so the declared foreign key is silently dropped. A fresh CREATE TABLE inlines the FK correctly (fixed by #1019), but the diff path that adds the column later does not.

SQLite does support ALTER TABLE t ADD COLUMN c INTEGER REFERENCES other(id) (for a NULL-able column, referenced table present), so the FK can and should be inlined on the ADD COLUMN.

This is distinct from #1037 (which is about changing references on an existing column): here the column is new, and its FK never reaches the generated SQL at all.

Reproduction (verified on 0.2.69)

// bun-query-builder SQLite FK-on-ADD-COLUMN repro. Run: bun add bun-query-builder && bun repro-issue.ts
import { Database } from 'bun:sqlite'
import { generateMigration, saveMigrationSnapshot } from 'bun-query-builder'
import { writeFileSync } from 'node:fs'

const DIR = './models-issue'
const server = `import { defineModel } from 'bun-query-builder'
export default defineModel({ name: 'Server', table: 'servers', primaryKey: 'id', autoIncrement: true,
  hasMany: ['Monitor'], attributes: { name: { validation: { rule: { name: 'string' } } } } })`
const monitor = (fk: boolean) => `import { defineModel } from 'bun-query-builder'
export default defineModel({ name: 'Monitor', table: 'monitors', primaryKey: 'id', autoIncrement: true,
  ${fk ? "belongsTo: ['Server']," : ''} attributes: {
    name: { validation: { rule: { name: 'string' } } },
    ${fk ? "serverId: { validation: { rule: { name: 'number' } } }," : ''}
  } })`

writeFileSync(`${DIR}/Server.ts`, server)
// BEFORE: monitors has no server_id. Snapshot this schema.
writeFileSync(`${DIR}/Monitor.ts`, monitor(false))
const before: any = await generateMigration(DIR, { dialect: 'sqlite', dryRun: true })
saveMigrationSnapshot(before.plan, { dialect: 'sqlite' })

// AFTER: add a belongsTo Server (monitors.server_id -> servers.id). Diff it.
writeFileSync(`${DIR}/Monitor.ts`, monitor(true))
const diff: any = await generateMigration(DIR, { dialect: 'sqlite', dryRun: true })
console.log('generated diff SQL:')
for (const s of diff.sqlStatements) console.log('   ', s.trim())

// Prove the FK is gone: build the schema and inspect.
const db = new Database(':memory:')
db.run('CREATE TABLE "servers" ("id" INTEGER PRIMARY KEY AUTOINCREMENT, "name" TEXT)')
db.run('CREATE TABLE "monitors" ("id" INTEGER PRIMARY KEY AUTOINCREMENT, "name" TEXT)')
for (const s of diff.sqlStatements) db.run(s)
const fks = db.query('PRAGMA foreign_key_list("monitors")').all()
console.log('\nmonitors foreign keys after applying the diff:', JSON.stringify(fks))
console.log(fks.length === 0 ? 'BUG: monitors.server_id -> servers.id was declared but NO foreign key exists.' : 'ok')

Output:

generated diff SQL:
    ALTER TABLE "monitors" ADD COLUMN "server_id" INTEGER;

monitors foreign keys after applying the diff: []
BUG: monitors.server_id -> servers.id was declared but NO foreign key exists.

Expected: ALTER TABLE "monitors" ADD COLUMN "server_id" INTEGER REFERENCES "servers"("id"); (the form SQLite accepts, matching the inline REFERENCES a fresh CREATE TABLE already emits). Actual: the REFERENCES clause is omitted; PRAGMA foreign_key_list("monitors") is empty.

Real-world impact

Found in a production Stacks app (StatusHQ). Its monitors.server_id → servers.id and four other belongsTo FKs were added to existing tables; on SQLite buddy migrate now warns:

5 of 32 declared foreign keys are missing from the live schema: monitors.server_id → servers.id, server_metric_samples.server_id → servers.id, incident_updates.user_id → users.id, team_members.user_id → users.id, sso_identities.user_id → users.id

The committed migration is exactly the buggy output: ALTER TABLE "monitors" ADD COLUMN "server_id" INTEGER; with a separate ADD CONSTRAINT … FOREIGN KEY file that the framework runner skips (SQLite can't ALTER … ADD CONSTRAINT). The net result is a column with no FK.

The same gap has a hard-error form

When the generator instead emits the constraint as an ALTER TABLE … ADD [CONSTRAINT …] FOREIGN KEY … statement (as it does for other dialects, and as older output did), SQLite rejects it outright:

new Database(':memory:')
  .run('ALTER TABLE "monitors" ADD CONSTRAINT "m_server_id_fk" FOREIGN KEY ("server_id") REFERENCES "servers"("id")')
// SQLiteError: near "CONSTRAINT": syntax error
// (and the name-less 'ADD FOREIGN KEY' form -> near "FOREIGN": syntax error)

That near "FOREIGN": syntax error is what surfaced in the app's error tracker during a migration run. So both the silent-missing-FK and the hard syntax error stem from one root: SQLite foreign keys cannot be added via ALTER; they must be inlined on ADD COLUMN (or applied via a 12-step table rebuild).

Suggested fix

For dialect: 'sqlite', when a diff adds a new column that carries a references, render it inline on the ADD COLUMN — ADD COLUMN "server_id" INTEGER REFERENCES "servers"("id") — reusing the same renderColumn reference logic that CREATE TABLE uses (#1019). Never emit ALTER TABLE … ADD CONSTRAINT/ADD FOREIGN KEY on SQLite; for cases that can't be inlined (adding/altering an FK on a pre-existing column, per #1037), use the table-rebuild that #1037 already notes.

Versions

  • bun-query-builder: reproduced on 0.2.69 (latest) and 0.2.59
  • Bun 1.x, SQLite via bun:sqlite

Related: #1037 (diff ignores references changes on existing columns), #1019 (CREATE TABLE inline REFERENCES), #1002 (belongsTo FK nullability).

8 days ago

Confirming this fix shipped, and correcting a wrong claim I made about it elsewhere.

Ran the repro from the issue body A/B against two installed copies - same script, same models, only the package version differing:

0.2.69  ALTER TABLE "monitors" ADD COLUMN "server_id" INTEGER;                             BROKEN
0.3.3   ALTER TABLE "monitors" ADD COLUMN "server_id" INTEGER REFERENCES "servers"("id");  FIXED

So closing this as completed was correct. Apologies for any noise - I had been about to ask for it to be re-opened on the strength of seeing addForeignKey(){return ""} still present in the 0.3.3 bundle. That method belongs to the SingleStore dialect, not SQLite: the one immediately after it throws Partial indexes (CompositeIndex.where) are not supported on SingleStore, and SingleStore has no foreign keys, so returning an empty string there is correct behaviour. I misattributed it while reading the minified dist.

For anyone arriving here from a downstream app still seeing the five-missing-FK warning: check your installed version. The fix is in 0.3.x, and an app pinned to 0.2.x will keep reporting it. In our case ./buddy migrate warns 5 of 32 declared foreign keys are missing from the live schema on 0.2.69, which is expected pre-fix rather than a regression.

One small thing that might be worth a doc line: on SQLite the pre-fix schemas cannot be repaired by re-running migrations, since the column already exists and SQLite cannot add a constraint by ALTER. Reaching the fixed behaviour on an existing database needs a table rebuild or a fresh migrate, not just the upgrade.

Sign in to comment on this issue.