From 9465e3b67593dcd841dbd3ebaf4c19bcf6b7c19f Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Tue, 5 May 2026 10:46:01 -0600 Subject: [PATCH] fix(migration): explicit NOT NULL on FK columns for SQLite (sc-105349) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- ..._2bee73611e32_composite_pk_association_tables.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/superset/migrations/versions/2026-05-01_23-36_2bee73611e32_composite_pk_association_tables.py b/superset/migrations/versions/2026-05-01_23-36_2bee73611e32_composite_pk_association_tables.py index e8a77614561..210a419d0ee 100644 --- a/superset/migrations/versions/2026-05-01_23-36_2bee73611e32_composite_pk_association_tables.py +++ b/superset/migrations/versions/2026-05-01_23-36_2bee73611e32_composite_pk_association_tables.py @@ -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: