From e8b73ec7e26c89df898e18cfe38855ec1586390d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 14:32:28 +0000 Subject: [PATCH] Remove transitional FK NOT DEFERRABLE convergence Foreign keys are created NOT DEFERRABLE by construction, so the one-time convergence path that flipped legacy DEFERRABLE FKs with a catalog-only ALTER CONSTRAINT has no remaining job. Drops SetConstraintNotDeferrableCorrection, DriftKind.DEFERRABLE and the ForeignKeyNameDrift member that used it, the deferrable comparison in _compare_foreign_keys, ConstraintState.deferrable, and the condeferrable column from the pg_constraint introspection query. The rename, replace, and validate corrections are unaffected. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WEvVhrMA6hqzxQGdN5AxWj --- plain-postgres/plain/postgres/connection.py | 5 +- .../plain/postgres/convergence/analysis.py | 30 +---- .../plain/postgres/convergence/corrections.py | 28 ----- .../plain/postgres/convergence/planning.py | 3 - .../plain/postgres/introspection/schema.py | 2 - plain-postgres/tests/conftest_convergence.py | 14 --- .../tests/internal/test_convergence_fk.py | 103 ------------------ 7 files changed, 7 insertions(+), 178 deletions(-) diff --git a/plain-postgres/plain/postgres/connection.py b/plain-postgres/plain/postgres/connection.py index e0e028ad21..477265aa3f 100644 --- a/plain-postgres/plain/postgres/connection.py +++ b/plain-postgres/plain/postgres/connection.py @@ -606,8 +606,7 @@ def get_constraints( WHERE fka.attrelid = c.confrelid AND fka.attnum = c.confkey[1]), c.convalidated, pg_get_constraintdef(c.oid), - c.confdeltype, - c.condeferrable + c.confdeltype FROM pg_constraint AS c JOIN pg_class AS cl ON c.conrelid = cl.oid WHERE cl.relname = %s AND pg_catalog.pg_table_is_visible(cl.oid) @@ -622,7 +621,6 @@ def get_constraints( validated, constraintdef, confdeltype, - condeferrable, ) in cursor.fetchall(): constraints[constraint] = { "columns": columns, @@ -632,7 +630,6 @@ def get_constraints( "definition": constraintdef, "validated": validated, "on_delete_action": confdeltype if kind == "f" else None, - "deferrable": condeferrable if kind == "f" else None, } # Now get indexes. Sort order, opclasses, INCLUDE, and predicates all # ride along inside `pg_get_indexdef` and are compared via the diff --git a/plain-postgres/plain/postgres/convergence/analysis.py b/plain-postgres/plain/postgres/convergence/analysis.py index 2576864447..66350056ce 100644 --- a/plain-postgres/plain/postgres/convergence/analysis.py +++ b/plain-postgres/plain/postgres/convergence/analysis.py @@ -53,7 +53,6 @@ class DriftKind(StrEnum): RENAMED = "renamed" UNDECLARED = "undeclared" UNVALIDATED = "unvalidated" - DEFERRABLE = "deferrable" @dataclass @@ -214,18 +213,16 @@ def describe(self) -> str: @dataclass class ForeignKeyNameDrift: - """An existing FK constraint to validate (UNVALIDATED), drop (UNDECLARED), - or make NOT DEFERRABLE (DEFERRABLE — Plain checks every FK immediately).""" + """An existing FK constraint to validate (UNVALIDATED) or drop + (UNDECLARED).""" table: str name: str - kind: Literal[DriftKind.UNVALIDATED, DriftKind.UNDECLARED, DriftKind.DEFERRABLE] + kind: Literal[DriftKind.UNVALIDATED, DriftKind.UNDECLARED] def describe(self) -> str: if self.kind is DriftKind.UNVALIDATED: return f"{self.table}: FK {self.name} NOT VALID" - if self.kind is DriftKind.DEFERRABLE: - return f"{self.table}: FK {self.name} DEFERRABLE" return f"{self.table}: FK {self.name} not declared" @@ -1315,8 +1312,8 @@ def _compare_foreign_keys( and cs.on_delete_action != on_delete.confdeltype ): # on_delete action mismatch — drop + re-add with new clause. - # The re-added constraint is NOT DEFERRABLE and gets validated, - # so this covers the other two cases as well. + # The re-added constraint gets validated, so this covers the + # unvalidated case as well. statuses.append( _fk_status( actual_name, @@ -1339,21 +1336,6 @@ def _compare_foreign_keys( ) continue - # Deferrable and NOT VALID are independent — report both so one - # converge pass fixes both. - if cs.deferrable: - # Older Plain releases created every FK DEFERRABLE INITIALLY - # DEFERRED. Flipping it is a catalog-only ALTER CONSTRAINT. - statuses.append( - _fk_status( - actual_name, - col, - issue="DEFERRABLE — Plain FKs are NOT DEFERRABLE", - drift=ForeignKeyNameDrift( - table=table, name=actual_name, kind=DriftKind.DEFERRABLE - ), - ) - ) if not cs.validated: statuses.append( _fk_status( @@ -1365,7 +1347,7 @@ def _compare_foreign_keys( ), ) ) - if not renamed and not cs.deferrable and cs.validated: + if not renamed and cs.validated: statuses.append(_fk_status(actual_name, col, issue=None, drift=None)) for name in sorted(actual.keys() - matched_fk_names): diff --git a/plain-postgres/plain/postgres/convergence/corrections.py b/plain-postgres/plain/postgres/convergence/corrections.py index 7dd2630ce3..0ad67014ed 100644 --- a/plain-postgres/plain/postgres/convergence/corrections.py +++ b/plain-postgres/plain/postgres/convergence/corrections.py @@ -370,34 +370,6 @@ def apply(self) -> str: return f"{replace_sql}; {validate_sql}" -@dataclass -class SetConstraintNotDeferrableCorrection(Correction): - """Make a DEFERRABLE FK constraint NOT DEFERRABLE. - - Older Plain releases created every FK DEFERRABLE INITIALLY DEFERRED. - ALTER CONSTRAINT only rewrites the catalog — the constraint stays - validated and nothing is scanned — but it does take a brief ACCESS - EXCLUSIVE lock on both tables, bounded by the usual lock_timeout. - Postgres only allows this on foreign keys. - """ - - pass_order = 2 - - table: str - name: str - - def describe(self) -> str: - return f"{self.table}: make FK {self.name} NOT DEFERRABLE" - - def apply(self) -> str: - sql = ( - f"ALTER TABLE {quote_name(self.table)}" - f" ALTER CONSTRAINT {quote_name(self.name)} NOT DEFERRABLE" - ) - _execute_and_commit(sql) - return sql - - @dataclass class SetNotNullCorrection(Correction): """Enforce NOT NULL via CHECK NOT VALID → VALIDATE → SET NOT NULL. diff --git a/plain-postgres/plain/postgres/convergence/planning.py b/plain-postgres/plain/postgres/convergence/planning.py index 7f5486ae58..aea6fa0f61 100644 --- a/plain-postgres/plain/postgres/convergence/planning.py +++ b/plain-postgres/plain/postgres/convergence/planning.py @@ -42,7 +42,6 @@ ReplaceForeignKeyCorrection, ResetStorageParameterCorrection, SetColumnDefaultCorrection, - SetConstraintNotDeferrableCorrection, SetNotNullCorrection, SetStorageParameterCorrection, ValidateConstraintCorrection, @@ -128,8 +127,6 @@ def _plan_drift(drift: Drift) -> PlanItem: return PlanItem(drift, ValidateConstraintCorrection(t, n)) case ForeignKeyNameDrift(kind=DriftKind.UNDECLARED, table=t, name=n): return PlanItem(drift, DropConstraintCorrection(t, n)) - case ForeignKeyNameDrift(kind=DriftKind.DEFERRABLE, table=t, name=n): - return PlanItem(drift, SetConstraintNotDeferrableCorrection(t, n)) case ForeignKeyRenameDrift(table=t, old_name=old, new_name=new): # Blocks sync, unlike other renames: the write path maps FK # violations back to the field by this name. diff --git a/plain-postgres/plain/postgres/introspection/schema.py b/plain-postgres/plain/postgres/introspection/schema.py index 5996fca3be..f720228461 100644 --- a/plain-postgres/plain/postgres/introspection/schema.py +++ b/plain-postgres/plain/postgres/introspection/schema.py @@ -82,7 +82,6 @@ class ConstraintState: target_table: str | None = None # FK only target_column: str | None = None # FK only on_delete_action: str | None = None # FK only: pg_constraint.confdeltype char - deferrable: bool = False # FK only: pg_constraint.condeferrable @dataclass @@ -152,7 +151,6 @@ def introspect_table( target_table=fk_target[0], target_column=fk_target[1], on_delete_action=info.get("on_delete_action"), - deferrable=info.get("deferrable", False), ) elif info.get("index"): indexes[name] = IndexState( diff --git a/plain-postgres/tests/conftest_convergence.py b/plain-postgres/tests/conftest_convergence.py index ac1848786e..6277500a86 100644 --- a/plain-postgres/tests/conftest_convergence.py +++ b/plain-postgres/tests/conftest_convergence.py @@ -38,20 +38,6 @@ def constraint_is_valid(table: str, name: str) -> bool: return row[0] if row else False -def constraint_is_deferrable(table: str, name: str) -> bool: - with get_connection().cursor() as cursor: - cursor.execute( - """ - SELECT c.condeferrable FROM pg_constraint c - JOIN pg_class cl ON c.conrelid = cl.oid - WHERE cl.relname = %s AND c.conname = %s - """, - [table, name], - ) - row = cursor.fetchone() - return row[0] if row else False - - def create_invalid_index( name: str, table: str = "examples_widget", diff --git a/plain-postgres/tests/internal/test_convergence_fk.py b/plain-postgres/tests/internal/test_convergence_fk.py index 034a7938b5..75bb9de45c 100644 --- a/plain-postgres/tests/internal/test_convergence_fk.py +++ b/plain-postgres/tests/internal/test_convergence_fk.py @@ -5,7 +5,6 @@ from app.examples.models.trees import TreeNode from conftest_convergence import ( constraint_exists, - constraint_is_deferrable, constraint_is_valid, execute, fk_on_delete_action, @@ -30,7 +29,6 @@ DropConstraintCorrection, RenameConstraintCorrection, ReplaceForeignKeyCorrection, - SetConstraintNotDeferrableCorrection, ValidateConstraintCorrection, ) from plain.postgres.utils import generate_fk_constraint_name @@ -178,7 +176,6 @@ def test_add_fk_creates_and_validates(self, isolated_db): assert "NOT VALID" in sql assert "VALIDATE CONSTRAINT" in sql - assert "DEFERRABLE" not in sql assert constraint_exists("examples_widgettag", widget_fk) assert constraint_is_valid("examples_widgettag", widget_fk) @@ -200,29 +197,6 @@ def test_validate_fk_after_add(self, isolated_db): assert constraint_is_valid("examples_widgettag", widget_fk) - def test_fk_is_not_deferrable(self, isolated_db): - """Convergence-created FK constraints are checked immediately.""" - widget_fk = generate_fk_constraint_name( - "examples_widgettag", "widget_id", "examples_widget", "id" - ) - - # Drop and recreate via convergence correction - fk_names = get_fk_constraint_names("examples_widgettag") - if widget_fk in fk_names: - execute(f'ALTER TABLE "examples_widgettag" DROP CONSTRAINT "{widget_fk}"') - - correction = AddForeignKeyCorrection( - table="examples_widgettag", - constraint_name=widget_fk, - column="widget_id", - target_table="examples_widget", - target_column="id", - on_delete_clause=" ON DELETE CASCADE", - ) - correction.apply() - - assert not constraint_is_deferrable("examples_widgettag", widget_fk) - def test_undeclared_fk_drop(self, isolated_db): """DropConstraintCorrection drops an undeclared FK.""" execute( @@ -457,7 +431,6 @@ def test_replace_fk_updates_action(self, isolated_db): assert constraint_exists("examples_childcascade", fk_name) assert constraint_is_valid("examples_childcascade", fk_name) - assert not constraint_is_deferrable("examples_childcascade", fk_name) assert fk_on_delete_action("examples_childcascade", fk_name) == "c" def test_on_delete_drift_planned_and_executed(self, isolated_db): @@ -503,82 +476,6 @@ def test_set_null_emits_set_null_clause(self, isolated_db): assert fk_on_delete_action("examples_childsetnull", fk_name) == "n" -class TestForeignKeyDeferrable: - """Plain FKs are checked immediately. A DEFERRABLE FK (older releases made - every FK DEFERRABLE INITIALLY DEFERRED) is drift, fixed with a catalog-only - ALTER CONSTRAINT — no revalidation scan.""" - - def test_detects_deferrable_fk(self, db): - fk_name = _recreate_fk( - "examples_childcascade", - "parent_id", - "examples_deleteparent", - "id", - clause=" ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED", - ) - - conn = get_connection() - with conn.cursor() as cursor: - analysis = analyze_model(conn, cursor, ChildCascade) - - fk_drifts = [d for d in analysis.drifts if isinstance(d, ForeignKeyDrift)] - assert fk_drifts == [ - ForeignKeyNameDrift( - table="examples_childcascade", - name=fk_name, - kind=DriftKind.DEFERRABLE, - ) - ] - - def test_deferrable_and_not_valid_converge_in_one_pass(self, isolated_db): - """A legacy FK left DEFERRABLE and NOT VALID (old add committed, its - validate didn't) needs both corrections in the same plan.""" - fk_name = _recreate_fk( - "examples_childcascade", - "parent_id", - "examples_deleteparent", - "id", - clause=" ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED NOT VALID", - ) - - conn = get_connection() - with conn.cursor() as cursor: - items = plan_model_convergence(conn, cursor, ChildCascade).executable() - assert [type(item.correction) for item in items] == [ - SetConstraintNotDeferrableCorrection, - ValidateConstraintCorrection, - ] - assert execute_plan(items).ok - - assert not constraint_is_deferrable("examples_childcascade", fk_name) - assert constraint_is_valid("examples_childcascade", fk_name) - - def test_deferrable_fk_made_not_deferrable(self, isolated_db): - fk_name = _recreate_fk( - "examples_childcascade", - "parent_id", - "examples_deleteparent", - "id", - clause=" ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED", - ) - - conn = get_connection() - with conn.cursor() as cursor: - items = plan_model_convergence(conn, cursor, ChildCascade).executable() - assert [type(item.correction) for item in items] == [ - SetConstraintNotDeferrableCorrection - ] - assert execute_plan(items).ok - - assert not constraint_is_deferrable("examples_childcascade", fk_name) - assert constraint_is_valid("examples_childcascade", fk_name) - assert fk_on_delete_action("examples_childcascade", fk_name) == "c" - - with conn.cursor() as cursor: - analysis = analyze_model(conn, cursor, ChildCascade) - assert [d for d in analysis.drifts if isinstance(d, ForeignKeyDrift)] == [] - - class TestForeignKeyRename: """RenameField/RenameModel leave the FK constraint under its old name. The write path maps a violation back to the field by the generated name,