re-introduce hardcoded per-dialect type argument comparison rules for specific cases such as PG VARCHAR length missing or not

[Palisand]

The bug Prior to alembic version 1.4.0, adding a length to a String type would be auto-detected and would result in a migration that included the change. Starting with version 1.4.0, this change is not included in a migration.

Expected behavior For a given column, adding length to types.String should result in a migration that includes the change.

To Reproduce

1. Install alembic < 1.4.0 (e.g. 1.3.3) and create an initial migration using autodetect. I used the following model:

from sqlalchemy import Column, Integer, String
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()


class User(Base):
    __tablename__ = 'user'

    id = Column(Integer, primary_key=True)
    name = Column(types.String)

2. Run alembic revision --autogenerate -m "initial" and observe that it produces something like the following migration:

from alembic import op
import sqlalchemy as sa


revision = 'dd4a6ee1bfd2'
down_revision = None
branch_labels = None
depends_on = None


def upgrade():
    op.create_table('user',
    sa.Column('id', sa.Integer(), nullable=False),
    sa.Column('name', sa.String(), nullable=True),
    sa.PrimaryKeyConstraint('id')
    )


def downgrade():
    op.drop_table('user')

3. Run alembic upgrade head, update types.String to types.String(10), and generate another migration with autogenerate. Observe something like the following migration:

from alembic import op
import sqlalchemy as sa


revision = 'cca9e3305312'
down_revision = 'dd4a6ee1bfd2'
branch_labels = None
depends_on = None


def upgrade():
    op.alter_column('user', 'name',
               existing_type=sa.VARCHAR(),
               type_=sa.String(length=10),
               existing_nullable=True)


def downgrade():
    op.alter_column('user', 'name',
               existing_type=sa.String(length=10),
               type_=sa.VARCHAR(),
               existing_nullable=True)

Repeat these steps for alembic >= 1.4.0 and you'll notice that the migration produced in step 3 is empty (there are no operations).

Note that if the String were to start out with a defined length, changing that length would result in a migration with the change for alembic >= 1.4.0.

Versions.

• OS: macOS Catalina (10.15.6)
• Python: 3.8
• Alembic: >= 1.4.0
• SQLAlchemy: 1.3.20
• Database: postgres

[zzzeek]

hi there -

this is by design. when a datatype does not have a certain parameter at all on one side of the comparison, and then it does have it on another, alembic is above all trying to avoid false positives, such as a parameter in the Python type that is ignored by a particular backend. to handle specific cases where a missing parameter is added or removed and there's no ambiguity that this is definitey intended would require the re-introduction of hardcoded rules on specific backends. if someone wants to work on this and can provide very good tests on a per-case basis we can accept PRs for such rules. otherwise alembic autogenerate does not give any guarantees of 100% correctness and always requires manual review.

[Palisand]

I see. Thank you for the reply and for not closing this issue.

I must add that another potential problem of comparing types in this manner occurs when changing the impl of a custom type. Take for instance a type with an impl of String that is later changed to String(x). Suppose this type was used for multiple columns. The change will not be auto-detected for any of these columns.

[zzzeek]

by "impl" of "custom type" are you referring to TypeDecorator? this sounds like a different issue.

[Palisand]

Yes, I mean TypeDecorator. What leads me to believe it is the same issue is that a custom type that starts out with String(x) and is changed to String(y) will be autodetected, but not when String is changed to String(x). The behavior is identical to what I presented in my first comment. The only difference is that the "true" type is wrapped.

[zzzeek]

oh, yes if it is String() to String(x) that is the issue, then that's the same problem. I dont see how the TypeDecorator aspect is significant.

[Palisand]

Sorry for the confusion. It's significant in the sense that if the custom type were to be used my many columns, an operation for each of those columns would have to be manually inserted to the migration.

[zzzeek]

you can change the constructor of your TypeDecorator to ensure the String has a length.

additionally, you can work around the entire issue by using a custom type comparison function as documented at https://alembic.sqlalchemy.org/en/latest/autogenerate.html#compare-types .