Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attribute type change in Mapped[] not detected #1247

Closed
josteinl opened this issue May 23, 2023 · 9 comments
Closed

Attribute type change in Mapped[] not detected #1247

josteinl opened this issue May 23, 2023 · 9 comments
Labels
autogenerate - detection awaiting info waiting for the submitter to give more information cant reproduce question usage and API questions

Comments

@josteinl
Copy link

Describe the bug

If I change the type of an attribute in my model, only defined by the Mapped annotation, then I do not get any changes in the resulting upgrade script:

class File(Base):

    # change type from str to int
    # size: Mapped[Optional[str]]   
    size: Mapped[Optional[int]]  
alembic revision -m "Change not detected"  --autogenerate

Not expected:

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    pass
    # ### end Alembic commands ###

Expected behavior

I expect an alter_column statement a la:

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.alter_column('file', 'size',
               existing_type=sa.NUMERIC(),
               nullable=True)
    # ### end Alembic commands ###

To Reproduce
I am very aware that this may be a problem on my side, and I think it is difficult to provide a complete example setup.

Versions.

  • OS: Windows 11
  • Python: 3.10.11
  • Alembic: 1.11.1
  • SQLAlchemy: 2.0.15
  • Database: postgis/postgis:11-2.5-alpine
  • DBAPI: psycopg2-binary 2.9.6

Additional context

Doing other changes to my model class, for instance, removing the Optional keyword or adding/removing/renaming, results in an expected change (except still no type change).

Have a nice day!

@josteinl josteinl added the requires triage New issue that requires categorization label May 23, 2023
@zzzeek zzzeek added question usage and API questions and removed requires triage New issue that requires categorization labels May 23, 2023
@zzzeek
Copy link
Member

zzzeek commented May 23, 2023

no reason this wouldn't work other than if you don't have compare_types configured, can you ensure you have that set to True?

@zzzeek
Copy link
Member

zzzeek commented May 23, 2023

can't reproduce locally, setting up version one, then changing as:

class File(Base):
    __tablename__ = "file"

    id: Mapped[int] = mapped_column(primary_key=True)

    # change type from str to int
    #size: Mapped[Optional[str]]
    size: Mapped[Optional[int]]

the Table is generated the same as always, Alembic does not see Mapped[] or anything like that, sees the change normally:

INFO  [alembic.autogenerate.compare] Detected type change from VARCHAR() to Integer() on 'file.size'
INFO  [sqlalchemy.engine.Engine] ROLLBACK
  Generating /home/classic/dev/alembic/foo/versions/450b718030fd_rev2.py ...  done

migration script:

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.alter_column('file', 'size',
               existing_type=sa.VARCHAR(),
               type_=sa.Integer(),
               existing_nullable=True)
    # ### end Alembic commands ###


def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.alter_column('file', 'size',
               existing_type=sa.Integer(),
               type_=sa.VARCHAR(),
               existing_nullable=True)
    # ### end Alembic commands ###

note compare_types=True

    with connectable.connect() as connection:
        context.configure(
            compare_type=True,
            connection=connection, target_metadata=target_metadata
        )

        with context.begin_transaction():
            context.run_migrations()

@CaselIT CaselIT added the awaiting info waiting for the submitter to give more information label May 23, 2023
@josteinl
Copy link
Author

Adding compare_type=True did solve my problem! 😃

I have used Alembic for years, but seldom do changes to my configuration when it is working. I was surprised that compare_type defaults to False? Encountered this problem after an upgrade attempt from SQLAlchemy version 1.4 to 2.0 using the new Mapped[] type on a large project. I can confirm that it behaves the same way on my old SQLAlchemy 1.4 branch. Seems that we do not change type that often.

Thank you so much @zzzeek for super fast help!

@zzzeek
Copy link
Member

zzzeek commented May 24, 2023

the type comparison logic was never considered to be that "stable" for many years and we did not want false positives . it was rewritten a few years ago so that false positives are very unlikely.

@CaselIT
Copy link
Member

CaselIT commented May 24, 2023

do you think it would make sense to flip that to true by default in 1.12?

@zzzeek
Copy link
Member

zzzeek commented May 24, 2023

seems scary

@CaselIT
Copy link
Member

CaselIT commented May 24, 2023

aren't false positive very unlikely (cit) now? :)

@zzzeek
Copy link
Member

zzzeek commented May 24, 2023

you're more the alembic maintainer now, fire at will

@CaselIT
Copy link
Member

CaselIT commented May 24, 2023

well, I've added #1248 so we can look into it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autogenerate - detection awaiting info waiting for the submitter to give more information cant reproduce question usage and API questions
Projects
None yet
Development

No branches or pull requests

3 participants