HR-ATS-Portal/docs/architecture/adr/0017-plain-sql-migrations-a...

23 KiB

ADR 0017 — Plain-SQL migrations are the schema authority; Django is the runner; two CI gates, not one

Status: Proposed — 2026-07-30. Ratification is task T-04, Phase 0 week 1, and it is a merge blocker: migration 001 does not merge while this ADR is Proposed.

Scope: Who authors DDL, what applies it, what managed is set to, what makemigrations --check means, and how objects Django cannot express are kept under review. Covers the whole schema — app, ref, audit, ai, staging.

Deciders: Talha Ahmed (the ruling, the SeparateDatabaseAndState migration template, the db/schema-ignore.toml contents, review of every migration). Ahmed Mujtaba (the second CI gate — build-from-migrations, pg_dump --schema-only capture, the trigger and column-privilege catalogue queries, the expected-dump diff and its failure output; plus the reference-data seed migrations that are the first real user of the template) — independently demonstrable, with a Talha review checkpoint.

This ADR is an arbitration document. It exists because one question was answered six different ways in six documents and ruled on nowhere. It supersedes the recommendations previously carried in 00-scope-classification.md §10 #1, 02-system-architecture.md §15 I1, 03-database-design.md §32.1 #1, 04-integrations-and-processing.md §9.1 #2, 05-security-rbac-ai-governance.md §9.1 I-1, 06-api-boundaries.md §9.1 #4 and 07-implementation-plan.md §17 #1, and the risk R-15. Those sections are retained as evidence and reasoning; none of them is a decision any more. Equivalent to _open-items.md RULING-02.


Context

What the repository contributes to this decision: nothing

Verified absent (_repo-findings.md §B): any migration directory, any migration tool, any ORM, query builder or database driver, requirements.txt / pyproject.toml, any Dockerfile, any CI configuration. The prototype's entire persistence layer is a seeded LCG generating 100 candidates in memory at page load (js/data.js:8-10, js/data.js:110-131), with localStorage used only for the theme preference (js/app.js:64,193,198).

So there is no migration history to inherit and no tooling to be compatible with. This is a free choice — which is precisely why it went undecided: nothing forced it, and every document could plausibly assume a different answer.

The contradiction, stated exactly

Two binding decisions in _decisions.md point in opposite directions:

Source Text Implication
Part 1, §"Backend language and framework" Django chosen partly because "26 modules with a junior need built-in migrations (none exist, §B)" ORM-generated migrations are a reason for the stack
Part 1, §"Testing, CI and delivery process" CI includes "a makemigrations --check gate so a model change cannot merge without its migration" The gate presupposes autogeneration
Part 2, §"Schema tooling and migration strategy" "Ordered, up-only plain-SQL migration files under db/migrations, applied by a thin runner… The ORM, if any, maps to the schema; it never generates it" Autogeneration is forbidden
Part 2, risk list (final bullet) Predicts this exact collision and asks for it to be "reconciled explicitly rather than discovered in implementation" It was neither, for six documents

Part 2 is binding on the database, so the authority question was already settled in text. What was never settled is the pair of questions that actually reach a developer's keyboard: what is Meta.managed set to, and what is the CI gate checking? Five documents answered those two questions inconsistently — one said managed = False "on every table carrying an invariant Django cannot express", another said managed = False "on nothing", and three described the CI gate in three different ways. That is the defect this ADR closes.

Why it cannot be deferred past migration 001

The choice determines the shape of the first migration file and the model layer above it. Discovering it at migration 40 means rewriting 40 files and re-deriving a schema dump from a database whose history nobody can reconstruct. It is a one-line policy with a very large late cost, which is why it is scheduled in Phase 0 week 1 and gated on the first migration PR rather than tracked as a risk.

The two groups of objects — the basis of the ruling

Every argument in this ADR turns on a distinction the earlier recommendations blurred:

Group Objects Django can autogenerate Django can declare Django can express at all
A Partial unique indexes with WHERE, expression indexes on lower(), CHECK with regex, GiST EXCLUDE no yesMeta.constraints / Meta.indexes yes
B plpgsql triggers (history, immutability, audit hash chain, search index), column-level GRANT/REVOKE, RANGE partitions and attach/detach, generated columns, DEFERRABLE INITIALLY DEFERRED constraint triggers, procrastinate's vendor migrations no no no

Group A can stay inside Django's own drift check. Group B cannot, at any layer, under any setting. A single gate therefore cannot watch the whole schema, and every recommendation that tried to make one gate suffice ended up either hiding Group A from review or pretending Group B did not exist.


Options considered

# Option Pros (at their strongest) Cons
A ORM-first. Django autogenerates structure; invariants added afterwards in hand-written RunSQL migrations Familiar, fast, makemigrations works as documented, and a junior can add a column unaided. Genuinely the cheapest path to a first table Autogeneration reorders and rewrites objects it did not create: a later autogenerated ALTER silently drops the column-level GRANTs that make audit_event append-only, and no gate notices. The schema is authored by a tool whose output nobody reads, so a security-relevant DDL change never appears in a reviewable diff. Directly contradicts a binding Part 2 decision
B Plain SQL as authority, Django as runner via SeparateDatabaseAndState + RunSQL, managed = True everywhere, two CI gates (chosen) One schema author (a human), one reviewable artefact (a SQL diff), one migration ledger. Group A stays declared in Meta so makemigrations --check remains a real gate rather than a permanently-silenced one; Group B gets a gate that can actually see it. Every object has exactly one gate watching it, and the ignore-list is explicit and reviewed rather than implicit Two gates to build and maintain, plus a committed pg_dump that must be regenerated in every migration PR. makemigrations autogeneration is given up, so adding a column means writing SQL and the mirroring state_operations. The state_operations mirror is hand-maintained and can be got wrong — the ORM gate catches that, but only if the mirror was attempted
C Hybrid: Django migrations own tables and columns; RunSQL owns constraints, triggers and grants — the recommendation four documents converged on Keeps makemigrations convenience for the boring 80% while putting the invariants in reviewed SQL. Looks like the pragmatic middle, which is why it was recommended repeatedly The schema is half-owned by two tools, which is worse than either owning it. Ordering breaks first: an autogenerated ALTER TABLE … RENAME cannot know about the partial unique index or the trigger a later RunSQL created against the old name, and column-level GRANTs must be re-applied after any autogenerated ALTER — a step nothing enforces. Reviewers cannot tell from a diff which tool owns a given object
D Plain SQL with managed = False on invariant-bearing tables, the position 02 §12.4 previously took Honest about the ORM not owning those tables, and stops makemigrations proposing changes to them without any ignore-list Backwards on exactly the wrong tables. managed = False removes a table from migration state, so ats_result, audit_event and every history table — the tables that must not change silently — leave the one gate watching them, and Django proposes nothing because it has been told the table is not its business. It also breaks TestCase table creation for those models, so the constraint tests (A-26) lose their fixtures
E Abandon Django's migration framework; run a standalone runner (Flyway, golang-migrate, Alembic in SQL-only mode), as _decisions.md Part 2 originally allowed Cleanest conceptual separation — the ORM is purely a mapper and has no opinion about schema at all. No state_operations mirror to maintain Two applied-state ledgers in one repository, and Django's test database creation, TestCase fixtures and migrate in local development all still want a migration graph. It also throws away the one Django feature Part 1 cited as a reason for the stack, for no gain over Option B
F No migrations at all in Phase 0 — a single schema.sql applied by docker compose, migrations introduced when the schema stabilises Genuinely faster while the schema churns daily, which it will in Phase 0 There is no such thing as a stable point at which migrations begin: staging exists from Phase 0 week 1 and carries data (07 W1 deliverable), so the first schema change after that is a manual ALTER nobody recorded. This is how schemas become unreproducible

Decision

Option B. The mechanism below is the canonical text. It lives in 02-system-architecture.md §12.4 and is quoted verbatim — never paraphrased — by 03 §32.1, 04 §9.1 #2, 05 §9.1 I-1 and 07 §17 #1. Four near-identical paraphrases is how the managed flag ended up pointing in two opposite directions across five documents; quoting is therefore a rule, not a style preference.

Migration authority ruling — canonical text (ADR 0017). Quote it; do not paraphrase it.

db/migrations/NNN_*.sql is the schema authority. Every Django migration is SeparateDatabaseAndState(database_operations=[RunSQL(<that file>)], state_operations=[…]), so Django owns ordering and the applied-state ledger and authors no DDL. Every model stays managed = True; managed = False is used on no table, because it would remove exactly the tables that carry invariants from the one gate watching them. makemigrations --check compares models against declared migration state — never against the live database — so it is kept as the model-vs-state gate, and it is kept quiet not by a flag but by declaring every object Django can model in Meta.constraints / Meta.indexes (CheckConstraint, UniqueConstraint(condition=…), Index(Lower(…)), ExclusionConstraint) and mirroring those same declarations in state_operations. Objects Django cannot model at all — triggers, column-level GRANT/REVOKE, RANGE partitions and their attach/detach, generated columns, DEFERRABLE INITIALLY DEFERRED constraint triggers, and procrastinate's vendor-managed migrations — are named in an explicit, reviewed db/schema-ignore.toml, and are covered instead by a second, SQL-level gate: CI builds a database by running every migration, captures pg_dump --schema-only --no-owner plus a catalogue query for triggers and column privileges, and diffs that against the committed expected dump; any difference fails the build, and updating the expected dump is a reviewed part of the migration PR. Two gates, two failure modes, neither one silently lying: the ORM gate catches a model that has drifted from state, the SQL gate catches a database object that no migration created — or that a migration created and nobody reviewed. Signed off as ADR 0017 before migration 001 is written; it restates the mechanism already binding in adr/0002-primary-relational-database.md §3.

Gate ownership — so neither developer has to guess

Object Declared in Meta In state_operations Watched by
Column, type, nullability, FK, plain index yes (implicitly, by the field) yes makemigrations --check
CHECK including regex (CheckConstraint) yes yes makemigrations --check
Partial unique index (UniqueConstraint(condition=…)) yes yes makemigrations --check
Expression index on lower() (Index(Lower(…))) yes yes makemigrations --check
GiST EXCLUDE (ExclusionConstraint) yes yes makemigrations --check
Trigger (history, immutability, audit hash chain, search index) no no pg_dump diff + catalogue query
Column-level GRANT/REVOKE (append-only ats_result, audit_event) no no catalogue query on information_schema.column_privileges
RANGE partition, ATTACH/DETACH no no pg_dump diff
Generated column no no pg_dump diff
DEFERRABLE INITIALLY DEFERRED constraint trigger no no pg_dump diff
procrastinate vendor migrations n/a — vendor-managed n/a ignore-listed explicitly, never by omission

Migration file template

# app/migrations/0006_job_version_immutability.py
from django.db import migrations, models
from pathlib import Path

SQL = (Path(__file__).resolve().parents[3] / "db/migrations/0006_job_version_immutability.sql")

class Migration(migrations.Migration):
    dependencies = [("app", "0005_job_version")]
    operations = [
        migrations.SeparateDatabaseAndState(
            database_operations=[migrations.RunSQL(SQL.read_text(), reverse_sql=None)],
            state_operations=[
                # Mirror ONLY what Django can model. Triggers and GRANTs from the .sql
                # file are absent here by design and are covered by the SQL gate.
                migrations.AddConstraint(
                    model_name="jobversion",
                    constraint=models.CheckConstraint(
                        check=models.Q(version_no__gte=1), name="ck_job_version_no_positive"
                    ),
                ),
            ],
        )
    ]

reverse_sql=None is deliberate: there are no down-migrations. Recovery from a bad migration is forward-fix plus PITR, per adr/0002 §3.

Non-negotiable rules

# Rule Enforced by
1 No DDL is authored by any tool. Every schema change is a hand-written, reviewed .sql file Talha reviews every migration; the SQL gate fails on any object no migration created
2 managed = False appears on no model, ever A CI grep; a violation is a failed build
3 Group A objects are declared in Meta and mirrored in state_operations makemigrations --check fails if the mirror is missing or wrong
4 db/schema-ignore.toml is explicit. An object is ignored because it is listed, never because it was forgotten Reviewed file; the SQL gate reads it and reports the ignore-list in its output
5 The committed expected pg_dump is regenerated in the same PR as the migration that changes it The SQL gate fails otherwise
6 Migrations run as a dedicated migration role, never the application role The application role has no DDL, and no UPDATE/DELETE on append-only tables
7 CREATE INDEX CONCURRENTLY on any table with real volume; expand/contract across three deploys; backfills are batched background jobs, never migration bodies Review, plus the deploy-time migration timeout

Justification

Why the authority question goes to Part 2. The load-bearing objects in this design are the invariants: raw_intake_id NOT NULL against a non-deferrable FK is what makes "raw intake before candidate" true; the GiST EXCLUDE is what makes overlapping history impossible; the column-level GRANTs are what make audit_event append-only against a psql session. None of that is autogeneratable, and a schema whose critical half is hand-written and whose other half is generated has no single reviewable form. One author, one artefact.

Why managed = True everywhere, reversing the earlier managed = False recommendation. This is the substantive change from what 02 §12.4 previously said, and it is worth being explicit about why the earlier position was wrong rather than quietly replacing it. managed = False was proposed to stop makemigrations proposing changes to tables whose invariants Django cannot see. It does stop that — by removing those tables from migration state entirely. The tables it would remove are ats_result, audit_event and every *_history table: exactly the append-only, invariant-bearing tables where a silent change is most dangerous. It also breaks Django's test-database creation for those models, which would take the A-26 constraint test suite with it. The correct way to keep the ORM gate quiet is to make the models true — declare what Django can declare — and to ignore-list what it cannot, explicitly, in a file a reviewer reads.

Why two gates rather than one. A gate that cannot see an object is not a gate; it is a claim. makemigrations --check compares models to declared migration state and never looks at the live database, so it can never detect a re-GRANTed UPDATE on audit_event, a dropped immutability trigger, or a partition that was attached by hand. Those are the failures with security consequences (05 §9.1 I-1 makes the same point from the controls side). The SQL gate is the only one that can see them, and it is cheap: CI already runs a real PostgreSQL service container (adr/0016), so building a database from migrations and dumping it costs seconds.

Why this is a good junior task. The second gate is a self-contained, independently demonstrable piece of work with a crisp definition of done and no dependency on the domain model: run migrations, capture a dump, query two catalogues, diff, print a readable failure. It is exactly the shape of task _repo-findings.md §I asks for — not CRUD, mechanically verifiable, and it teaches the schema.

The honest cost. Adding a nullable column becomes: write the .sql, write the model field, mirror it in state_operations, regenerate the expected dump. That is four steps where ORM-first is one. For a schema whose constraints are the product requirements, and where a silent DDL change is a compliance incident, four steps is the correct price. We are not claiming it is free.


Consequences

Positive

  • One schema author and one reviewable artefact: every schema change is a SQL diff in a PR.
  • Every object in the schema has exactly one named gate watching it, and the mapping is a table above rather than folklore.
  • The ORM gate stays meaningful instead of being permanently silenced by a flag.
  • The append-only guarantees on ats_result and audit_event become continuously verified rather than asserted once in a migration nobody re-reads.
  • One migration ledger, so migrate, test-database creation and local docker compose all work normally.
  • The six scattered recommendations collapse to one citable ruling, which is what R-04's bus-factor mitigation actually requires.

Negative — the costs being accepted

  • makemigrations autogeneration is given up. Four steps per column change, not one.
  • The state_operations mirror is hand-maintained. Getting it wrong is caught by the ORM gate, but only if it was attempted; a developer who writes SQL and no mirror gets a passing ORM gate and a model that lies. Rule 3 plus review is the only defence, and it is a real residual risk.
  • A committed pg_dump must be regenerated in migration PRs, and dump noise (ordering, extension version strings) will cause some spurious failures until the capture is normalised.
  • Two CI gates to build and maintain, with the second one owned by the junior.
  • pg_dump output differs across PostgreSQL minor versions, so the CI container version must be pinned exactly, not by major version alone.

Risks

# Risk Likelihood Mitigation
R1 A migration adds an object and nobody updates the expected dump, so the gate fails and the fix becomes "regenerate blindly" — turning review into rubber-stamping High The gate prints the diff, not just a failure. Talha reviews the dump diff as part of the migration; a dump change with no corresponding .sql line is an automatic request for changes
R2 state_operations drifts from the .sql file: the SQL is right, the mirror is stale, and the ORM gate passes Medium The ORM gate catches model-vs-state drift, so this only survives if the model and the mirror are both wrong in the same way. Rule 3 is a review checklist item on every migration PR
R3 Dump noise makes the SQL gate flaky and the team starts ignoring it Medium Pin the PostgreSQL minor version; normalise the capture (--no-owner, sorted catalogue queries, stripped version comments) before the gate becomes required; treat a flaky gate as a P1 bug on the gate, not a reason to disable it
R4 Ratification slips past the first migration and migration 001 lands under an unstated policy Low It is a merge blocker on migration 001 (T-04), not a best-effort target
R5 procrastinate's vendor migrations change on upgrade and the SQL gate fails for a reason nobody owns Medium Vendor tables are ignore-listed explicitly in db/schema-ignore.toml; a queue upgrade is a deliberate PR that updates the ignore-list and the dump together
R6 The four-step column change is felt as friction and someone reintroduces autogeneration for "just this one table" Medium Rule 2's CI grep for managed = False plus the SQL gate: an autogenerated ALTER produces a dump diff with no .sql file behind it, which fails the build

Revisit conditions

Reopen this ADR if any of the following becomes true:

  1. Django gains first-class migration support for triggers, column-level privileges and partition attach/detach — at which point the second gate could collapse into the first.
  2. The state_operations mirror is empirically the top source of migration defects after ~50 migrations, which would argue for Option E (drop Django's migration graph entirely) rather than back toward Option A.
  3. The invariant set shrinks to things an ORM can express — which would mean the design's database-enforced guarantees had been abandoned, and is a much larger decision than this one.
  4. A third developer joins and schema authorship stops being reviewable by one person.

  • adr/0002-primary-relational-database.md §3 — the binding schema-tooling mechanism this ADR restates and operationalises.
  • adr/0016-real-postgres-in-ci.md — the real-PostgreSQL CI container both gates depend on. Neither gate is buildable against SQLite.
  • adr/0004-background-job-queue.mdprocrastinate's vendor-managed migrations, ignore-listed explicitly under rule 4.
  • _decisions.md §Rulings, and _open-items.md RULING-02 — the arbitration entry equivalent to this ADR.
  • 02-system-architecture.md §12.4 — the canonical text's home. Quote from there.
  • 07-implementation-plan.md T-04 (ratification, Phase 0 week 1), R-15 (the risk this closes), §17 #1 (evidence).