diff --git a/CLAUDE.md b/CLAUDE.md index 7a23dc7345..1a8d6739d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -149,6 +149,21 @@ code stops using it. Removing a schema object is therefore a two-step process ac 1. **Release 1:** Stop using the object in the code, but leave it in the schema. No drop migration yet. 2. **Release 2:** Drop the object in a migration, now that no running code (current or N-1) references it. +**"Stop using" an ORM column means stopping both reads _and_ writes.** SQLAlchemy eagerly includes every +mapped column in its default `SELECT`, and eagerly emits any column that has a Python-side `default=` in every +`INSERT`. So leaving a column mapped — even when no application code reads or assigns it — does **not** stop +using it, and a later drop will break the still-running previous release. To fully stop using a column in +release 1, before dropping it in release 2: + +- **Reads:** mark the column `deferred()` so it is excluded from the default `SELECT`. +- **Writes:** remove any Python-side `default=`. If the column is `NOT NULL`, removing the Python default + alone makes inserts fail the not-null constraint (SQLAlchemy sends no value and the database has none), so + first move the default into the database with a `server_default`. A **nullable** column with no default + is already omitted from `INSERT`s, so it needs no write-side change. + +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**. + 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 diff --git a/alembic/versions/20260715_dbeb42224c1b_add_server_default_to_lanes_size.py b/alembic/versions/20260715_dbeb42224c1b_add_server_default_to_lanes_size.py new file mode 100644 index 0000000000..2efbb323b9 --- /dev/null +++ b/alembic/versions/20260715_dbeb42224c1b_add_server_default_to_lanes_size.py @@ -0,0 +1,29 @@ +"""Add server_default to lanes.size + +Revision ID: dbeb42224c1b +Revises: a6c85605404c +Create Date: 2026-07-15 13:42:28.828886+00:00 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "dbeb42224c1b" +down_revision = "a6c85605404c" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ``lanes.size`` is deprecated and no longer read or written by the current + # code, but it is NOT NULL with no database default. Moving the default into + # the database lets the ORM omit the column from INSERTs (it previously + # supplied a Python-side ``default=0``), so a follow-up release can drop the + # column without breaking the still-running previous release's inserts. + op.alter_column("lanes", "size", server_default=sa.text("0")) + + +def downgrade() -> None: + op.alter_column("lanes", "size", server_default=None) diff --git a/src/palace/manager/sqlalchemy/model/lane.py b/src/palace/manager/sqlalchemy/model/lane.py index cab6284e25..b090a6c431 100644 --- a/src/palace/manager/sqlalchemy/model/lane.py +++ b/src/palace/manager/sqlalchemy/model/lane.py @@ -13,11 +13,13 @@ Unicode, UniqueConstraint, or_, + text, ) from sqlalchemy.dialects.postgresql import ARRAY, INT4RANGE, JSON from sqlalchemy.ext.associationproxy import association_proxy from sqlalchemy.orm import ( Mapped, + deferred, relationship, ) from sqlalchemy.orm.session import Session @@ -115,8 +117,9 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): # The code that maintained them (update_size and the lane-size Celery tasks) # has been removed; the columns remain mapped only so the model continues to # match the database schema, and will be dropped in a follow-up migration. - size: Mapped[int] = Column(Integer, nullable=False, default=0) - size_by_entrypoint = Column(JSON, nullable=True) + # TODO: Drop these next release when it is safe to do so. + size = deferred(Column(Integer, nullable=False, server_default=text("0"))) + size_by_entrypoint = deferred(Column(JSON, nullable=True)) # A lane may have one parent lane and many sublanes. sublanes: Mapped[list[Lane]] = relationship(