fix(migration): explicit NOT NULL on FK columns for SQLite (sc-105349)

Found by running fresh-install + round-trip against a real SQLite DB:
6 of the 8 affected tables had FK columns that were originally
declared nullable. PostgreSQL and MySQL implicitly promote the
constituent columns of an ``ALTER TABLE ... ADD PRIMARY KEY`` to
``NOT NULL``; SQLite does not (it's a long-standing SQLite quirk —
only ``INTEGER PRIMARY KEY`` enforces NOT NULL on a composite-PK
column). Result: a fresh SQLite install would accept
``INSERT INTO dashboard_slices (NULL, 5)`` despite both columns
being part of the composite PK.

Our integration tests previously masked this: the test fixture seeds
columns with ``nullable=False``, so the post-upgrade NOT NULL
assertion passed regardless of whether the migration enforced it.

Fix: add explicit ``batch_op.alter_column(fk, nullable=False)`` for
both FK columns inside the per-table batch_alter_table block. On
PostgreSQL and MySQL this is a no-op (PK already implies NOT NULL);
on SQLite it adds the missing NOT NULL declaration so a fresh
install matches the data-model.md "After" contract.

Verified end-to-end:
- Postgres + MySQL: column shape unchanged (still NOT NULL)
- SQLite fresh install + round-trip: all 8 tables now have NOT NULL
  on FK columns, ``INSERT (NULL, 5)`` correctly rejected with
  IntegrityError on dashboard_slices, dashboard_user, sqlatable_user

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mike Bridge
2026-05-05 10:46:01 -06:00
parent 65a3491861
commit 9465e3b675

View File

@@ -310,10 +310,23 @@ def upgrade() -> None:
) as batch_op:
batch_op.drop_column("id")
batch_op.create_primary_key(f"pk_{t.name}", [t.fk1, t.fk2])
# SQLite quirk: composite PRIMARY KEY does not promote the
# constituent columns to NOT NULL (only ``INTEGER PRIMARY
# KEY`` does). PostgreSQL and MySQL implicitly promote the
# PK columns to NOT NULL when the constraint is added,
# so the explicit ``alter_column`` is a no-op on those
# backends but enforces the post-upgrade contract on
# SQLite. Without it, ``INSERT (NULL, 5)`` would succeed
# on SQLite despite the columns being part of the PK.
batch_op.alter_column(t.fk1, existing_type=sa.Integer, nullable=False)
batch_op.alter_column(t.fk2, existing_type=sa.Integer, nullable=False)
else:
with op.batch_alter_table(t.name) as batch_op:
batch_op.drop_column("id")
batch_op.create_primary_key(f"pk_{t.name}", [t.fk1, t.fk2])
# See comment above re: SQLite composite-PK NOT NULL quirk.
batch_op.alter_column(t.fk1, existing_type=sa.Integer, nullable=False)
batch_op.alter_column(t.fk2, existing_type=sa.Integer, nullable=False)
def downgrade() -> None: