db: rename the four double-prefixed CHECK constraints (#3275)
Build images / sign-extension (push) Successful in 4s
CI / lint (push) Failing after 5s
CI / extension-version (push) Successful in 5s
Build images / build-agent (push) Successful in 9s
CI / frontend-build (push) Successful in 20s
CI / backend-lint-and-test (push) Successful in 33s
Build images / build-ml (push) Successful in 44s
Build images / build-web (push) Successful in 41s
CI / integration (push) Successful in 3m52s
Build images / sign-extension (push) Successful in 4s
CI / lint (push) Failing after 5s
CI / extension-version (push) Successful in 5s
Build images / build-agent (push) Successful in 9s
CI / frontend-build (push) Successful in 20s
CI / backend-lint-and-test (push) Successful in 33s
Build images / build-ml (push) Successful in 44s
Build images / build-web (push) Successful in 41s
CI / integration (push) Successful in 3m52s
Run 5026 got the models-vs-chain diff to 7 lines. Three findings, and one
of them reverses an assumption I made in the previous commit.
The doubled CHECK names are what the DATABASE has, not what the generator
invented. base.py's convention is ck_%(table_name)s_%(constraint_name)s,
which — unlike uq/fk/ix — applies even to a constraint that already has a
name, so four migrations that passed an already-prefixed name got it
prefixed twice:
ck_import_settings_ck_import_settings_singleton
ck_ml_settings_ck_ml_settings_singleton
ck_post_ck_post_translation_override
ck_tag_ck_tag_fandom_requires_character
The workflow repair added last commit is still correct and still needed —
autogenerate really does re-double a name on the round trip — but it was
making the MODELS side clean against a chain that is dirty. The
comment in ml_settings.py claiming its bare name "matches migration 0003"
was simply false; 0003 produces the doubled form.
Nothing reads a CHECK constraint by name, so this has never done harm.
But it is precisely the development-era residue the collapsed baseline
exists to leave behind, and a public schema should not ship it — so 0088
renames the deployed constraints and all six models now declare bare
names. RENAME CONSTRAINT is catalog-only: no scan, no rewrite, no
revalidation, which is why this is safe on post and tag. Guarded on
pg_constraint scoped by conrelid, so it is a no-op on a database built
from the models.
ix_tag_fandom_id showed as a difference only because chain_ref was pinned
to 0a5bbe8, which predates 0088 — the comparison was measuring the models
against a chain missing the migration that closes the gap. chain_ref now
defaults to blank, meaning "the chain in this ref". Pin it to a commit
only after the collapse, when the tree no longer carries the revisions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017QHszn9H8VBvx5Ke8x1hvw
This commit is contained in:
@@ -34,9 +34,9 @@ on:
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
chain_ref:
|
||||
description: 'Commit/tag that still carries the full 0001..0087 chain'
|
||||
description: 'Commit/tag carrying the full chain; blank = this ref (use a pinned commit only AFTER the collapse)'
|
||||
type: string
|
||||
default: '0a5bbe8'
|
||||
default: ''
|
||||
mode:
|
||||
description: 'chain = compare against this tree''s migrations; models = compare against a schema built from the MODELS'
|
||||
type: string
|
||||
@@ -100,10 +100,15 @@ jobs:
|
||||
- name: Build the schema the OLD chain produces
|
||||
env:
|
||||
CHAIN_REF: ${{ github.event.inputs.chain_ref }}
|
||||
THIS_SHA: ${{ github.sha }}
|
||||
run: |
|
||||
set -eux
|
||||
docker exec "$PG_CONTAINER" createdb -U fabledcurator fc_chain
|
||||
git worktree add /tmp/chain "$CHAIN_REF"
|
||||
# Blank means "the chain in this ref", which is what you want while
|
||||
# the chain is still intact — comparing the models against a PINNED
|
||||
# older commit reports every migration written since as a difference.
|
||||
# Pin it only after the collapse, when the tree no longer has them.
|
||||
git worktree add /tmp/chain "${CHAIN_REF:-$THIS_SHA}"
|
||||
ls /tmp/chain/alembic/versions/*.py | wc -l
|
||||
cd /tmp/chain
|
||||
DB_NAME=fc_chain alembic upgrade head
|
||||
|
||||
@@ -6,8 +6,8 @@ schema disagreed. Almost all of them were the MODEL being wrong — missing
|
||||
migration — and those are fixed in the model files with no DDL at all, because
|
||||
the database already had them.
|
||||
|
||||
This migration carries the remainder: the one case where the MODEL was right
|
||||
and the database was missing something.
|
||||
This migration carries the remainder — the two places where DDL is actually
|
||||
needed, because the database is what is wrong.
|
||||
|
||||
`tag.fandom_id` is declared `index=True` on the model, but no migration ever
|
||||
created that index. Every autogenerate run since would have proposed adding
|
||||
@@ -29,6 +29,29 @@ guarantee built from different objects, which is why the two schemas did not
|
||||
line up. The model now declares the constraint and the plain index separately,
|
||||
so it describes what is actually there. No DDL is needed for it.
|
||||
|
||||
Also here: four CHECK constraints whose names carry their table prefix TWICE.
|
||||
|
||||
`base.py`'s naming convention is `ck_%(table_name)s_%(constraint_name)s`, and
|
||||
unlike the uq/fk/ix entries it applies even to a constraint that already has a
|
||||
name. Four migrations passed an already-prefixed name, so the convention
|
||||
prefixed it again:
|
||||
|
||||
ck_import_settings_ck_import_settings_singleton
|
||||
ck_ml_settings_ck_ml_settings_singleton
|
||||
ck_post_ck_post_translation_override
|
||||
ck_tag_ck_tag_fandom_requires_character
|
||||
|
||||
Nothing reads a CHECK constraint by name, so this has never done any harm —
|
||||
but it is exactly the development-era residue the collapsed baseline exists to
|
||||
leave behind, and a public schema should not ship it. The models now declare
|
||||
bare names, which the convention renders into the single-prefix form; this
|
||||
renames the deployed constraints to match.
|
||||
|
||||
RENAME CONSTRAINT is a catalog-only operation: no table scan, no rewrite, no
|
||||
validation of existing rows. It takes a brief ACCESS EXCLUSIVE lock and
|
||||
returns. That is why this is safe to do on `post` and `tag`, which are the two
|
||||
large tables in the schema.
|
||||
|
||||
Revision ID: 0088
|
||||
Revises: 0087
|
||||
Create Date: 2026-08-30
|
||||
@@ -43,6 +66,38 @@ down_revision: Union[str, None] = "0087"
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
# (table, doubled name, single-prefix name)
|
||||
DOUBLED_CHECKS = (
|
||||
("import_settings", "ck_import_settings_ck_import_settings_singleton",
|
||||
"ck_import_settings_singleton"),
|
||||
("ml_settings", "ck_ml_settings_ck_ml_settings_singleton",
|
||||
"ck_ml_settings_singleton"),
|
||||
("post", "ck_post_ck_post_translation_override",
|
||||
"ck_post_translation_override"),
|
||||
("tag", "ck_tag_ck_tag_fandom_requires_character",
|
||||
"ck_tag_fandom_requires_character"),
|
||||
)
|
||||
|
||||
|
||||
def _rename_check(table: str, old: str, new: str) -> None:
|
||||
# Guarded on pg_constraint rather than run bare: a database built from the
|
||||
# models (a fresh install, or the CI integration schema) already has the
|
||||
# single-prefix name, and this migration must be a no-op there rather than
|
||||
# an error. Same reasoning as the CREATE INDEX IF NOT EXISTS below.
|
||||
op.execute(
|
||||
f"""
|
||||
DO $$
|
||||
BEGIN
|
||||
IF EXISTS (
|
||||
SELECT 1 FROM pg_constraint
|
||||
WHERE conname = '{old}' AND conrelid = '{table}'::regclass
|
||||
) THEN
|
||||
ALTER TABLE {table} RENAME CONSTRAINT {old} TO {new};
|
||||
END IF;
|
||||
END $$;
|
||||
"""
|
||||
)
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
# IF NOT EXISTS because the index is what the model already asks for: any
|
||||
@@ -50,6 +105,12 @@ def upgrade() -> None:
|
||||
# and this migration must be a no-op there rather than an error.
|
||||
op.execute("CREATE INDEX IF NOT EXISTS ix_tag_fandom_id ON tag (fandom_id)")
|
||||
|
||||
for table, old, new in DOUBLED_CHECKS:
|
||||
_rename_check(table, old, new)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
for table, old, new in DOUBLED_CHECKS:
|
||||
_rename_check(table, new, old)
|
||||
|
||||
op.execute("DROP INDEX IF EXISTS ix_tag_fandom_id")
|
||||
|
||||
@@ -43,11 +43,15 @@ class ExternalLink(Base):
|
||||
# needs its constraint swapped in the same migration (#3275).
|
||||
CheckConstraint(
|
||||
"host IN ('mega', 'gdrive', 'mediafire', 'dropbox', 'pixeldrain')",
|
||||
name="ck_external_link_host",
|
||||
# Bare name: Base.metadata's naming convention prepends
|
||||
# ck_<table>_. Pre-prefixing it here doubles the prefix — see
|
||||
# alembic 0088, which renames the four constraints that shipped
|
||||
# that way (#3275).
|
||||
name="host",
|
||||
),
|
||||
CheckConstraint(
|
||||
"status IN ('pending', 'downloading', 'downloaded', 'failed', 'skipped', 'dead')",
|
||||
name="ck_external_link_status",
|
||||
name="status",
|
||||
),
|
||||
# One row per (post, url). The full url (incl. #fragment) is the identity
|
||||
# — the same file linked twice in a post collapses to one row.
|
||||
|
||||
@@ -22,6 +22,11 @@ class ImportSettings(Base):
|
||||
__tablename__ = "import_settings"
|
||||
# Bare constraint name — Base.metadata's naming convention applies the
|
||||
# ck_<table>_<name> prefix, producing the final ck_import_settings_singleton.
|
||||
# Bare name — Base.metadata's naming convention prepends ck_<table>_,
|
||||
# producing ck_import_settings_singleton. The chain shipped the DOUBLED
|
||||
# ck_import_settings_ck_import_settings_singleton, because the migration
|
||||
# pre-prefixed the name and the convention prefixed it again; alembic
|
||||
# 0088 renames it to what this line has always produced (#3275).
|
||||
__table_args__ = (CheckConstraint("id = 1", name="singleton"),)
|
||||
|
||||
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
||||
|
||||
@@ -21,7 +21,10 @@ from .base import Base
|
||||
class MLSettings(Base):
|
||||
__tablename__ = "ml_settings"
|
||||
# Bare name — Base.metadata's naming convention prepends ck_<table>_,
|
||||
# producing the final ck_ml_settings_singleton (matches migration 0003).
|
||||
# producing ck_ml_settings_singleton. The chain shipped the DOUBLED
|
||||
# ck_ml_settings_ck_ml_settings_singleton, because the migration
|
||||
# pre-prefixed the name and the convention prefixed it again; alembic
|
||||
# 0088 renames it to what this line has always produced (#3275).
|
||||
__table_args__ = (CheckConstraint("id = 1", name="singleton"),)
|
||||
|
||||
id: Mapped[int] = mapped_column(Integer, primary_key=True)
|
||||
|
||||
@@ -41,7 +41,11 @@ class Post(Base):
|
||||
UniqueConstraint("source_id", "external_post_id", name="uq_post_source_external_id"),
|
||||
CheckConstraint(
|
||||
"translation_override IN ('auto', 'force', 'original')",
|
||||
name="ck_post_translation_override",
|
||||
# Bare name: Base.metadata's naming convention prepends
|
||||
# ck_<table>_. Pre-prefixing it here doubles the prefix — see
|
||||
# alembic 0088, which renames the four constraints that shipped
|
||||
# that way (#3275).
|
||||
name="translation_override",
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
@@ -83,7 +83,11 @@ class Tag(Base):
|
||||
unique=True),
|
||||
CheckConstraint(
|
||||
"(fandom_id IS NULL) OR (kind = 'character')",
|
||||
name="ck_tag_fandom_requires_character",
|
||||
# Bare name: Base.metadata's naming convention prepends
|
||||
# ck_<table>_. Pre-prefixing it here doubles the prefix — see
|
||||
# alembic 0088, which renames the four constraints that shipped
|
||||
# that way (#3275).
|
||||
name="fandom_requires_character",
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user