From 4e2be49fc34eff7741a74aaf24532595ff6d08cb Mon Sep 17 00:00:00 2001 From: Daniel Bernstein Date: Wed, 16 Sep 2026 13:53:29 -0700 Subject: [PATCH 1/2] Document how to retire a whole table in the online-migration rules The online-migration section explained the two-release split for columns (deferred, server_default) but not for tables, and the missing guidance is what let PR #3521 reach review with a drop that would have broken N-1 webservers. Two traps were undocumented: a relationship() is a read, because SQLAlchemy loads it on every parent delete to cascade or to null the child's foreign key; and a fresh database's schema comes from create_all over the models rather than from replaying migrations, so deleting a model class removes its table from new installs regardless of the migration. Together they put the release boundary between the relationship and the model, not around the model. Also note the two follow-on details that bit us: removing the relationship makes surviving rows block parent deletes, and N-1's test suite must not contain tests that write to the doomed table. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 9b776e9219..09e4265f06 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -167,6 +167,41 @@ release 1, before dropping it in release 2: Both the read-side (`deferred`) and write-side (`server_default`) changes are backwards-compatible, so they belong together in **release 1**; the drop is **release 2**. +**Dropping a whole table: the split cuts at the relationship, not the model.** Two things make a table +harder to stop using than the column rules above suggest: + +- **A `relationship()` is a read.** SQLAlchemy loads a relationship whenever its parent is deleted — to + cascade the delete (`mapper.cascade_iterator`), or to null the child's foreign key + (`dependency.presort_deletes`). So leaving `Parent.children` mapped does **not** stop using the child + table, even when no application code ever touches the attribute: every `session.delete(parent)` still + SELECTs from it. Release 1 has to delete the `relationship()` definitions themselves, and the matching + `back_populates` on the other side — retiring the code that *used* them is not enough. +- **A fresh schema is built from the models, not by replaying migrations.** + `InstanceInitializationScript.initialize_database_schema` calls `SessionManager.initialize_schema` + (`metadata.create_all`) and then stamps alembic head. Deleting the model class therefore drops the table + out of every newly initialized database immediately, whatever the migrations say — and the + backwards-compatibility gate builds its "current" schema exactly this way. + +Together these put the release boundary between the relationship and the model: + +1. **Release 1:** remove the relationships and their `back_populates`, and delete the tests that exercise the + model. **Keep the model class**, so the table still exists in fresh schemas. +2. **Release 2:** remove the model class and drop the table in a migration. + +Splitting at the model instead — release 1 deletes the class, release 2 drops the table — fails, because +release 1 already removes the table from new installs while N-1 still maps the relationships. + +Two details that are easy to miss in release 1: + +- **Deleting the relationship can break parent deletes.** It was the relationship that cascaded (or nulled + the child FK) by hand; once it is gone only the database's own rules apply. A child foreign key declared + without an `ON DELETE` clause will reject the parent delete while any row survives. Either empty the table + in the release-1 migration (right when the rows are dead data — `TRUNCATE` avoids the row-level WAL of a + large `DELETE`, and its `ACCESS EXCLUSIVE` lock is uncontended on a table nothing reads) or give the + foreign key an `ON DELETE` clause. A FK that already declares `ON DELETE CASCADE` needs neither. +- **Delete the model's tests in release 1 too.** The gate runs N-1's *test suite* against the new schema, so + tests that build rows in the doomed table fail in release 2 even though no application code would. + The same constraint applies in reverse when adding required schema: a new non-nullable column must first be added as nullable, or with a **server default** so the database fills it in for rows written by N-1 code (which does not yet know about the column). Never write a single migration that both adds a not-yet-used column as From 6840c94c56f72bbf1742b0beaff4bcd2bddf4846 Mon Sep 17 00:00:00 2001 From: Daniel Bernstein Date: Wed, 23 Sep 2026 16:49:24 -0700 Subject: [PATCH 2/2] Correct the release-1 table cleanup guidance: prefer DELETE over TRUNCATE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release-1 migration runs while N-1 is still serving, and N-1 still maps the relationship — so it still SELECTs the child table on every parent delete. Calling TRUNCATE's ACCESS EXCLUSIVE lock "uncontended on a table nothing reads" contradicted that, and would have pointed maintainers at a statement that can queue every later query on the table behind it. Recommend a plain DELETE (ROW EXCLUSIVE, never blocks N-1's reads; the WAL and bloat are irrelevant on a table dropped next release, with a lock_timeout-guarded TRUNCATE as the fallback for tables too large for one statement. Co-Authored-By: Claude Opus 5 EOF ) --- CLAUDE.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 09e4265f06..5350b668dc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -195,10 +195,17 @@ Two details that are easy to miss in release 1: - **Deleting the relationship can break parent deletes.** It was the relationship that cascaded (or nulled the child FK) by hand; once it is gone only the database's own rules apply. A child foreign key declared - without an `ON DELETE` clause will reject the parent delete while any row survives. Either empty the table - in the release-1 migration (right when the rows are dead data — `TRUNCATE` avoids the row-level WAL of a - large `DELETE`, and its `ACCESS EXCLUSIVE` lock is uncontended on a table nothing reads) or give the - foreign key an `ON DELETE` clause. A FK that already declares `ON DELETE CASCADE` needs neither. + without an `ON DELETE` clause will reject the parent delete while any row survives. Fix it either by + giving the foreign key an `ON DELETE` clause, or by emptying the table in the release-1 migration — and if + you empty it, use a plain `DELETE`, not `TRUNCATE`. The table is not quiet yet at that point: the + migration runs while N-1 is still serving, and N-1 still maps the relationship, so it still SELECTs the + child table on every parent delete. `TRUNCATE` takes an `ACCESS EXCLUSIVE` lock, which conflicts with + those reads, and a `TRUNCATE` left waiting queues every later query on the table behind it — an online + migration turned into a stall. A `DELETE` takes only `ROW EXCLUSIVE`, never blocks them, and its extra + WAL and dead rows don't matter on a table that is dropped next release anyway. If the table really is too + large for one `DELETE`, a `TRUNCATE` guarded by a short `lock_timeout` + (`op.execute("SET lock_timeout = '2s'")` first) at least fails the migration fast instead of stalling + traffic. A FK that already declares `ON DELETE CASCADE` needs none of this. - **Delete the model's tests in release 1 too.** The gate runs N-1's *test suite* against the new schema, so tests that build rows in the doomed table fail in release 2 even though no application code would.