Skip to content

Commit 6282776

Browse files
anth-volkclaude
andcommitted
Name the check constraints as their table definitions do
Revision 0002 passed full check constraint names, such as ck_simulations_kind, which the naming convention prefixed a second time, so the database held ck_simulations_ck_simulations_kind while the table definitions name it ck_simulations_kind. alembic check does not compare check constraints, so nothing noticed; a later revision dropping one by its defined name would have failed. The revision now marks the names as final (op.f), and a test compares every constraint name in the migrated database with the names the table definitions give. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 33d5c85 commit 6282776

2 files changed

Lines changed: 53 additions & 12 deletions

File tree

‎libs/policyengine-simulation-persistence/tests/test_migration_cycle.py‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import re
56
import time
67

78
import pytest
@@ -18,10 +19,13 @@
1819
HEAD_REVISION,
1920
SIMULATION_SCHEMA,
2021
create_simulation_engine,
22+
simulation_metadata,
2123
)
2224
from report_support import economy_pair, economy_report, report_store
2325
from sqlalchemy import create_engine, inspect, text
26+
from sqlalchemy.dialects import postgresql
2427
from sqlalchemy.exc import OperationalError, ProgrammingError
28+
from sqlalchemy.schema import CreateTable
2529

2630
pytestmark = pytest.mark.postgres
2731

@@ -229,3 +233,36 @@ def test_the_runtime_role_cannot_change_or_delete_a_definition(
229233
connection.execute(text(statement))
230234
finally:
231235
engine.dispose()
236+
237+
238+
def _declared_constraint_names() -> set[str]:
239+
"""Return the names the table definitions give their constraints."""
240+
names: set[str] = set()
241+
for table in simulation_metadata.sorted_tables:
242+
ddl = str(CreateTable(table).compile(dialect=postgresql.dialect()))
243+
names.update(re.findall(r"CONSTRAINT (\w+)", ddl))
244+
return names
245+
246+
247+
def test_every_constraint_has_the_name_its_definition_gives_it(
248+
migrated_database,
249+
) -> None:
250+
"""Compare names here, since alembic check does not compare check constraints."""
251+
engine = _superuser_engine(migrated_database)
252+
try:
253+
with engine.connect() as connection:
254+
database_names = set(
255+
connection.execute(
256+
text(
257+
"SELECT conname FROM pg_constraint "
258+
"JOIN pg_namespace ON pg_namespace.oid = connamespace "
259+
"WHERE nspname = 'simulation' "
260+
"AND contype IN ('c', 'f', 'p', 'u') "
261+
"AND conrelid <> 'simulation.alembic_version'::regclass"
262+
)
263+
).scalars()
264+
)
265+
finally:
266+
engine.dispose()
267+
268+
assert database_names == _declared_constraint_names()

‎migrations/versions/0002_simulations_and_reports.py‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,27 +54,29 @@ def _create_simulations() -> None:
5454
),
5555
sa.CheckConstraint(
5656
"kind IN ('economy', 'household')",
57-
name="ck_simulations_kind",
57+
name=op.f("ck_simulations_kind"),
58+
),
59+
sa.CheckConstraint(
60+
"country ~ '^[a-z]{2}$'", name=op.f("ck_simulations_country")
5861
),
59-
sa.CheckConstraint("country ~ '^[a-z]{2}$'", name="ck_simulations_country"),
6062
sa.CheckConstraint(
6163
"(kind = 'household' AND household_id IS NOT NULL "
6264
"AND region_code IS NULL AND dataset_name IS NULL) "
6365
"OR (kind = 'economy' AND household_id IS NULL "
6466
"AND region_code IS NOT NULL)",
65-
name="ck_simulations_population_matches_kind",
67+
name=op.f("ck_simulations_population_matches_kind"),
6668
),
6769
sa.CheckConstraint(
6870
"region_code IS NULL OR region_code ~ '^\\S+$'",
69-
name="ck_simulations_region_code",
71+
name=op.f("ck_simulations_region_code"),
7072
),
7173
sa.CheckConstraint(
7274
"dataset_name IS NULL OR dataset_name ~ '^\\S+$'",
73-
name="ck_simulations_dataset_name",
75+
name=op.f("ck_simulations_dataset_name"),
7476
),
7577
sa.CheckConstraint(
7678
"legacy_simulation_id IS NULL OR legacy_simulation_id > 0",
77-
name="ck_simulations_legacy_simulation_id",
79+
name=op.f("ck_simulations_legacy_simulation_id"),
7880
),
7981
sa.ForeignKeyConstraint(
8082
["policy_id", "country"],
@@ -136,19 +138,21 @@ def _create_reports() -> None:
136138
server_default=sa.text("now()"),
137139
nullable=False,
138140
),
139-
sa.CheckConstraint("kind IN ('economy', 'household')", name="ck_reports_kind"),
140-
sa.CheckConstraint("country ~ '^[a-z]{2}$'", name="ck_reports_country"),
141-
sa.CheckConstraint("year BETWEEN 1900 AND 2200", name="ck_reports_year"),
142141
sa.CheckConstraint(
143-
"jsonb_typeof(options) = 'object'", name="ck_reports_options"
142+
"kind IN ('economy', 'household')", name=op.f("ck_reports_kind")
143+
),
144+
sa.CheckConstraint("country ~ '^[a-z]{2}$'", name=op.f("ck_reports_country")),
145+
sa.CheckConstraint("year BETWEEN 1900 AND 2200", name=op.f("ck_reports_year")),
146+
sa.CheckConstraint(
147+
"jsonb_typeof(options) = 'object'", name=op.f("ck_reports_options")
144148
),
145149
sa.CheckConstraint(
146150
"kind = 'household' OR reform_simulation_id IS NOT NULL",
147-
name="ck_reports_economy_has_reform",
151+
name=op.f("ck_reports_economy_has_reform"),
148152
),
149153
sa.CheckConstraint(
150154
"legacy_report_id IS NULL OR legacy_report_id > 0",
151-
name="ck_reports_legacy_report_id",
155+
name=op.f("ck_reports_legacy_report_id"),
152156
),
153157
sa.ForeignKeyConstraint(
154158
["baseline_simulation_id", "country", "kind"],

0 commit comments

Comments
 (0)