Constraint renaming in batch operations (Possible incompatibility with sqlalchemy v2.0.45)

[mbergen]

Used alembic version: 1.16.1 and 1.18.5

After upgrading sqlalchemy to version 2.0.45, one of my alembic migrations generated different SQL for constraints names.
It fails to rename an existing constraint to a new table name, which means that further migrations might fail to drop said constraint as it it still referring to the old name. Is my usage wrong?

The migration in question looks like this:

from alembic import op
import sqlalchemy as sa

# [alembic revision identifiers]

CONSTRAINT_NAMING_CONVENTION = {
    "ix": "ix_%(table_name)s_%(column_0_label)s",
    "uq": "uq_%(table_name)s_%(column_0_name)s",
    "ck": "ck_%(table_name)s_%(constraint_name)s",
    "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
    "pk": "pk_%(table_name)s"
}

def upgrade() -> None:
    op.rename_table('OldTableName', 'TableName')
    with op.batch_alter_table('TableName', schema=None, naming_convention=CONSTRAINT_NAMING_CONVENTION) as batch_op:
        batch_op.alter_column('column1', new_column_name='column2')

in version 2.0.44, the generated SQL reads like this:

ALTER TABLE "OldTableName" RENAME TO "_alembic_tmp_TableName";
CREATE TABLE "_alembic_tmp_TableName' (
        column2 VARCHAR NOT NULL, 
        CONSTRAINT "pk_TableName" PRIMARY KEY (column2), 
        CONSTRAINT "fk_TableName_Other_name_Other" FOREIGN KEY(column2) REFERENCES "Other" (name) ON DELETE CASCADE
);
INSERT [...];
ALTER TABLE "_alembic_tmp_TableName" RENAME TO "TableName";

Now in version 2.0.45 and newer, it reads like this:

ALTER TABLE "OldTableName" RENAME TO "_alembic_tmp_TableName";
CREATE TABLE "_alembic_tmp_TableName' (
        column2 VARCHAR NOT NULL, 
        CONSTRAINT "pk_OldTableName" PRIMARY KEY (column2), 
        CONSTRAINT "fk_OldTableName_Other_name_Other" FOREIGN KEY(column2) REFERENCES "Other" (name) ON DELETE CASCADE
);
INSERT [...];
ALTER TABLE "_alembic_tmp_TableName" RENAME TO "TableName";

I assume this is related to either sqlalchemy#12954 or sqlalchemy#12924. See also other changes in sqlalchemy v2.0.45

In addition, when trying to use batch_op.create_forgeign_key to manually create the missing constraint, there is no SQL rendered for a constraint at all.

[zzzeek]

odd because it's not like we dont have tests for renaming constraints in batch mode.

though i guess the issue here is that the naming convention is using the older name. will have to see if we have regression tests for that.

[mbergen]

Could you link me one of these tests? I'm happy to investigate if there are any differences to my setup.

[zzzeek]

the batch tests are at https://github.com/sqlalchemy/alembic/blob/main/tests/test_batch.py .

I have GLM5.2 whipping through this though and it already has a theory for what's going wrong.

[zzzeek]

this bug exists prior to SQLAlhcemy 2.0.45 for lowercase names. the uppercase name here is acting differently due to the above mentioned SQLAlhcemy fixes, so it basically normalizes the bug for all table names.

[zzzeek]

(edit: claude generated comment)

Thanks for the detailed report. I dug into this and split it out into a tracked issue: #1834.

Short version: this is an alembic batch-mode bug (surfaced, not caused, by the SQLAlchemy 2.0.45 SQLite reflection fix #12954). When a naming_convention is passed to batch_alter_table(), batch mode should re-derive the recreated table's constraint names from that convention against the new table name, but it currently copies the reflected names verbatim — so after a rename_table() the constraints keep the old table name. It's also not actually version-specific: with an all-lowercase constraint name you get the stale name on every SQLAlchemy version including 2.0.44. Full analysis and MCVE are in #1834.

[mbergen]

Thanks for looking into this!
Another question I had:
As I understand now, the name of constraints is only adjusted when one of the columns in it is.
That means if I rename the table, and alter a column that is not part of one of the constraints, I'm still stuck with the outdated constraints names unless I manually alter them (which didn't seem to work in the sense that batch_op.create_foreign_key didn't produce any constraints in the generated SQL).

Should there be an addition to the current options?

• an option to rename_table, that already adjusts the constraints?
• (an option to rename during an existing batch operation?)

[zzzeek]

yes

[CaselIT]

ok. maybe a reapply_naming_convention is a better flag name, since that's applied also to indexes, but it's likely a nitpick

I'm kinda wondering why batch_alter_table doesn't have a new_table_name argument too

[zzzeek]

guess it could

[mbergen]

is actually not what would happen if we werent doing a batch operation. those constraint names would not actually change

Does a normal alter_table not adjust the constraint name? Might just bet the bug with quoted table names that is throwing me off, but i think it did in one of my tests.

im not saying what you are looking for here isn't useful but perhaps it should not be by default.

I don't mind adapting how my migrations look like, i'm just not sure what the best way for it is.
I tried to adjust my migration to include manual constraint changes during the batch operation, but that only worked for dropping constraints, not adding them (not sure if due to misuse or another bug)

what would you think of an API like this to get the behavior you've up til now been using "off label" ? :

IMO it would only make sense together with the option to rename a table in a batch operation; as then the constraint name change is where it "belongs". And then it could even be considered to be the default.

[zzzeek]

is actually not what would happen if we werent doing a batch operation. those constraint names would not actually change

Does a normal alter_table not adjust the constraint name? Might just bet the bug with quoted table names that is throwing me off, but i think it did in one of my tests.

no, ALTER TABLE where you rename the table would never have an effect on the names of the constraints. even if they wanted it to, it could not because the database knows nothing about the naming convention templates we have in our python code.

im not saying what you are looking for here isn't useful but perhaps it should not be by default.

I don't mind adapting how my migrations look like, i'm just not sure what the best way for it is. I tried to adjust my migration to include manual constraint changes during the batch operation, but that only worked for dropping constraints, not adding them (not sure if due to misuse or another bug)

well the way it's been working in every case except for "SQlite with mixed case conventions" is to leave those names intact. a rename table is not maybe that common of an operation. but the batch process, since it recreates everything anyway, gives us a neat place to make this actually happen since it is what people would often (if not usually) want.

what would you think of an API like this to get the behavior you've up til now been using "off label" ? :

IMO it would only make sense together with the option to rename a table in a batch operation; as then the constraint name change is where it "belongs". And then it could even be considered to be the default.

OK maybe like @CaselIT mentioned, we have a new_table_name argument