fix(schema): correct ON DELETE behavior for 5 drifted FKs (#1126) by jakebromberg · Pull Request #1411 · WXYC/Backend-Service

jakebromberg · 2026-06-14T04:36:31Z

Closes #1126.

Summary

Five FK constraints on wxyc_schema.flowsheet, wxyc_schema.rotation, and wxyc_schema.reviews were created with ON DELETE NO ACTION in 0000_rare_prima.sql and recreated unchanged by 0016_nervous_hydra.sql, but the Drizzle schema source declares them as:

Table	Column	References	Migration says	schema.ts / snapshot says
flowsheet	show_id	shows.id	NO ACTION	SET NULL
flowsheet	album_id	library.id	NO ACTION	SET NULL
flowsheet	rotation_id	rotation.id	NO ACTION	SET NULL
rotation	album_id	library.id	NO ACTION	CASCADE
reviews	album_id	library.id	NO ACTION	CASCADE

Because meta/0093_snapshot.json already records the schema-source values, drizzle-kit generate produced no fix migration — the drift was invisible to the normal authoring loop. New environments diverge from prod and deleting a parent row in shows or library raises 23503 foreign_key_violation the application code is written to expect would be auto-handled.

Changes

shared/database/src/migrations/0094_fix-fk-on-delete-flowsheet-rotation-reviews.sql — DROP + ADD each of the five FKs with the intended ON DELETE behaviour. Follows the same shape as 0048_fix-fk-on-delete-set-null.sql (the predecessor that patched the analogous drift for schedule / shift_covers / shows.primary_dj_id per FK constraints on schedule, shift_covers, and shows use NO ACTION instead of SET NULL #433; this migration is the missed remainder).
shared/database/src/migrations/meta/_journal.json — append idx 94, when = previous + 1ms.
shared/database/src/migrations/meta/0094_snapshot.json — byte-identical to 0093 except for id / prevId (no schema change; only DB-side action is patched).
shared/database/src/migrations/meta/applied-hashes.json — npm run drizzle:freeze-hashes.
tests/integration/fk-on-delete-flowsheet-rotation-reviews.spec.js — asserts pg_constraint.confdeltype for all five FKs against the live DB and exercises the SET NULL behaviour via a parent-row DELETE inside a rolled-back transaction. Complements the existing tests/unit/database/schema.fk-cascades.test.ts schema-side guard.

schema.ts is unchanged because the source already declares the intended onDelete actions — the drift is purely between migration history and schema.

Verification

# After the migration applies:
psql -h 127.0.0.1 -U wxyc -d wxyc_db -c "
  SELECT conname, confdeltype
  FROM pg_constraint
  WHERE conname IN (
    'flowsheet_show_id_shows_id_fk',
    'flowsheet_album_id_library_id_fk',
    'flowsheet_rotation_id_rotation_id_fk',
    'rotation_album_id_library_id_fk',
    'reviews_album_id_library_id_fk'
  );"
# Expect: 'n' for the three flowsheet FKs, 'c' for rotation + reviews.

Test plan

npm run lint:migrations — passes (93 entries → now 94; only the historical idx-47 duplicate warning).
npm run format:check — passes.
npm run lint — 0 errors (525 pre-existing warnings unchanged).
npm run typecheck — passes.
npm run test:unit — 3151 passed (228 suites), including the predecessor schema.fk-cascades.test.ts which asserts the schema-side declaration.
CI migrate-dryrun job — will fire automatically (paths-filter db-init matches shared/database/src/migrations/**). The dry-run restores the latest prod RDS snapshot and runs scripts/dryrun-migrate.mjs, exercising the new migration against prod-shaped data.
CI integration-tests — will run the new spec against the per-worker schema in Docker.

Schema constraint shape report

data-shape report errored (exit 0): node:internal/modules/runmain:107 triggerUncaughtException( ^ Error ERRMODULENOTFOUND: Cannot find package 'postgres' imported from /home/runner/work/Backend-Service/Backend-Service/scripts/schema-shape-report.mjs Did you mean to import "postgres/cjs/src/index.js"? at Object.getPackageJ; manual check required

…complete integration coverage Apply five review findings on the #1126 FK ON DELETE drift fix: 1. Migration 0094 now uses ADD CONSTRAINT ... NOT VALID instead of a bare ADD CONSTRAINT. The bare form would have taken AccessExclusiveLock on flowsheet (~857k rows) for the full validation-scan duration, blocking on-air DJ writes during deploy. NOT VALID makes the ADD metadata-only and instant. Drizzle's migrator wraps the whole migration in one transaction (drizzle-orm/pg-core/dialect.js:60), so an in-migration VALIDATE would defeat the lock benefit — VALIDATE is documented as an out-of-band operator step in the header instead. For these five constraints the validation is effectively a no-op anyway: the existing NO ACTION FK already kept the reference relation consistent, and only the ON DELETE action is changing (forward-looking). 2. Integration spec now exercises all three ON DELETE actions in the fix (flowsheet SET NULL, rotation CASCADE, reviews CASCADE) instead of only the flowsheet SET NULL path. The previous spec ran two tests: a static pg_constraint.confdeltype assertion plus a single behavioural test. The behavioural test silently never ran because its library INSERT omitted four NOT NULL columns (artist_id, genre_id, format_id, code_number) and its flowsheet INSERT omitted play_order — so the spec was a false green for the actual cascade/SET-NULL behaviour. 3. Spec now uses the shared getTestDb() pool + withRollback helper from tests/utils/db.js instead of opening its own postgres() connection and rolling back via a hand-written intentional-error idiom. Matches the convention used by neighbouring specs (album-metadata-upsert, enrichment-worker-claim, library-identity-backfill). 4. Inserts now use the seed_db.sql baseline rows (artist 1, genre 11, format 1) to satisfy library's NOT NULL FK columns rather than threading FK resolution through the test. Sequence-assigned ids land at 7200+ (shape fixture sets library_id_seq to 7199), safely above the explicit-id fixture range. Local CI: lint, format:check, typecheck, test:unit all pass. Migration dry-run not available locally (Docker daemon not running); will be covered by CI's migrate-dryrun job since the change touches db-init paths.

Five FK constraints on `wxyc_schema.flowsheet`, `wxyc_schema.rotation`, and `wxyc_schema.reviews` were created with `ON DELETE NO ACTION` in `0000_rare_prima.sql` and recreated unchanged by `0016_nervous_hydra.sql`, but the Drizzle schema source declares them as `SET NULL` (flowsheet) and `CASCADE` (rotation, reviews). Because `meta/0093_snapshot.json` already records the schema-source values, `drizzle-kit generate` produced no fix migration — the drift was invisible to the normal authoring loop. Migration 0094 drops + recreates the five FKs with the intended ON DELETE behaviour, following the pattern in `0048_fix-fk-on-delete-set-null.sql` (the predecessor that patched the analogous drift for `schedule`, `shift_covers`, `shows.primary_dj_id` per #433). The schema source already encodes the desired state, so no `schema.ts` change is needed; the new snapshot is byte-identical to 0093 except for `id` / `prevId`. A new integration spec (`tests/integration/fk-on-delete-flowsheet-rotation-reviews.spec.js`) asserts `pg_constraint.confdeltype` for all five FKs and exercises the SET NULL behaviour via a parent-row DELETE inside a rolled-back transaction. Complements the existing `tests/unit/database/schema.fk-cascades.test.ts` schema-side guard. Closes #1126.

…complete integration coverage Apply five review findings on the #1126 FK ON DELETE drift fix: 1. Migration 0094 now uses ADD CONSTRAINT ... NOT VALID instead of a bare ADD CONSTRAINT. The bare form would have taken AccessExclusiveLock on flowsheet (~857k rows) for the full validation-scan duration, blocking on-air DJ writes during deploy. NOT VALID makes the ADD metadata-only and instant. Drizzle's migrator wraps the whole migration in one transaction (drizzle-orm/pg-core/dialect.js:60), so an in-migration VALIDATE would defeat the lock benefit — VALIDATE is documented as an out-of-band operator step in the header instead. For these five constraints the validation is effectively a no-op anyway: the existing NO ACTION FK already kept the reference relation consistent, and only the ON DELETE action is changing (forward-looking). 2. Integration spec now exercises all three ON DELETE actions in the fix (flowsheet SET NULL, rotation CASCADE, reviews CASCADE) instead of only the flowsheet SET NULL path. The previous spec ran two tests: a static pg_constraint.confdeltype assertion plus a single behavioural test. The behavioural test silently never ran because its library INSERT omitted four NOT NULL columns (artist_id, genre_id, format_id, code_number) and its flowsheet INSERT omitted play_order — so the spec was a false green for the actual cascade/SET-NULL behaviour. 3. Spec now uses the shared getTestDb() pool + withRollback helper from tests/utils/db.js instead of opening its own postgres() connection and rolling back via a hand-written intentional-error idiom. Matches the convention used by neighbouring specs (album-metadata-upsert, enrichment-worker-claim, library-identity-backfill). 4. Inserts now use the seed_db.sql baseline rows (artist 1, genre 11, format 1) to satisfy library's NOT NULL FK columns rather than threading FK resolution through the test. Sequence-assigned ids land at 7200+ (shape fixture sets library_id_seq to 7199), safely above the explicit-id fixture range. Local CI: lint, format:check, typecheck, test:unit all pass. Migration dry-run not available locally (Docker daemon not running); will be covered by CI's migrate-dryrun job since the change touches db-init paths.

The static-assertion test in fk-on-delete-flowsheet-rotation-reviews.spec.js was failing CI with `op ANY/ALL (array) requires array on right side` because `sql.array(arr)` (without a type argument) does not bind the value as a Postgres text[] in postgres-js. The canonical idiom in this codebase (flowsheet-etl-setwhere.spec.js:61, album-metadata-upsert.spec.js:159, enrichment-worker-sweep.spec.js:63) is `ANY(${arr})` with a bare JS array — postgres-js infers the array type from the values. This test was failing on the previous CI run too (pre-review-fix); this commit clears the inherited bug.

…complete integration coverage Apply five review findings on the #1126 FK ON DELETE drift fix: 1. Migration 0094 now uses ADD CONSTRAINT ... NOT VALID instead of a bare ADD CONSTRAINT. The bare form would have taken AccessExclusiveLock on flowsheet (~857k rows) for the full validation-scan duration, blocking on-air DJ writes during deploy. NOT VALID makes the ADD metadata-only and instant. Drizzle's migrator wraps the whole migration in one transaction (drizzle-orm/pg-core/dialect.js:60), so an in-migration VALIDATE would defeat the lock benefit — VALIDATE is documented as an out-of-band operator step in the header instead. For these five constraints the validation is effectively a no-op anyway: the existing NO ACTION FK already kept the reference relation consistent, and only the ON DELETE action is changing (forward-looking). 2. Integration spec now exercises all three ON DELETE actions in the fix (flowsheet SET NULL, rotation CASCADE, reviews CASCADE) instead of only the flowsheet SET NULL path. The previous spec ran two tests: a static pg_constraint.confdeltype assertion plus a single behavioural test. The behavioural test silently never ran because its library INSERT omitted four NOT NULL columns (artist_id, genre_id, format_id, code_number) and its flowsheet INSERT omitted play_order — so the spec was a false green for the actual cascade/SET-NULL behaviour. 3. Spec now uses the shared getTestDb() pool + withRollback helper from tests/utils/db.js instead of opening its own postgres() connection and rolling back via a hand-written intentional-error idiom. Matches the convention used by neighbouring specs (album-metadata-upsert, enrichment-worker-claim, library-identity-backfill). 4. Inserts now use the seed_db.sql baseline rows (artist 1, genre 11, format 1) to satisfy library's NOT NULL FK columns rather than threading FK resolution through the test. Sequence-assigned ids land at 7200+ (shape fixture sets library_id_seq to 7199), safely above the explicit-id fixture range. Local CI: lint, format:check, typecheck, test:unit all pass. Migration dry-run not available locally (Docker daemon not running); will be covered by CI's migrate-dryrun job since the change touches db-init paths.

jakebromberg mentioned this pull request Jun 14, 2026

FK ON DELETE drifted: flowsheet, rotation, reviews mismatch schema vs DB #1126

Closed

5 tasks

jakebromberg added 3 commits June 14, 2026 14:06

jakebromberg force-pushed the fix/1126-fk-on-delete-drift branch from 813d1c2 to b55f2fc Compare June 14, 2026 21:10

jakebromberg merged commit 317be3c into main Jun 14, 2026
6 checks passed

jakebromberg deleted the fix/1126-fk-on-delete-drift branch June 14, 2026 21:20

jakebromberg mentioned this pull request Jun 15, 2026

[Tracker] BS open-issue triage — P0 sequencing, cross-cluster conflicts #645

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(schema): correct ON DELETE behavior for 5 drifted FKs (#1126)#1411

fix(schema): correct ON DELETE behavior for 5 drifted FKs (#1126)#1411
jakebromberg merged 3 commits into
mainfrom
fix/1126-fk-on-delete-drift

jakebromberg commented Jun 14, 2026

Uh oh!

github-actions Bot commented Jun 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jakebromberg commented Jun 14, 2026

Summary

Changes

Verification

Test plan

Related

Uh oh!

github-actions Bot commented Jun 14, 2026

Schema constraint shape report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant