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

existing_type option for alter_column in batch does not work with Variant #982

Closed
THS-on opened this issue Jan 31, 2022 · 4 comments
Closed
Labels
batch migrations bug Something isn't working great mcve an issue with a great mcve

Comments

@THS-on
Copy link

THS-on commented Jan 31, 2022

Describe the bug
Batch altering a column with the existing_type option set to a Variant like sa.Text().with_variant(sa.Text(10000), "mysql") fails with an error in alembic newer alembic versions starting with version 1.7.0.

Expected behavior
Migrations still work with a Variant set as existing type. In this case sa.Text().with_variant(sa.Text(10000), "mysql").

To Reproduce

  1. create a new project with: alembic init alembic
  2. create alembic/versions/f9828e772e26_init.py with the following content
"""init

Revision ID: f9828e772e26
Revises: 
Create Date: 2022-01-31 18:08:27.382366

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = 'f9828e772e26'
down_revision = None
branch_labels = None
depends_on = None


def upgrade():
    op.create_table("example", sa.Column('text', sa.Text().with_variant(sa.Text(10000), "mysql")))
    # This is done because sqlite does not support renaming a column
    with op.batch_alter_table('example') as batch_op:
        batch_op.alter_column('text', new_column_name='text_new',
                              existing_type=sa.Text().with_variant(sa.Text(10000), "mysql"), existing_nullable=True)

def downgrade():
    pass
  1. change sqlalchemy.url in alembic.ini to sqlite:///test.sqlite
  2. run migrations with: alembic upgrade head

Error

INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> f9828e772e26, init
Traceback (most recent call last):
  File "<user_dir>venv/bin/alembic", line 8, in <module>
    sys.exit(main())
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/config.py", line 588, in main
    CommandLine(prog=prog).main(argv=argv)
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/config.py", line 582, in main
    self.run_cmd(cfg, options)
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/config.py", line 559, in run_cmd
    fn(
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/command.py", line 320, in upgrade
    script.run_env()
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/script/base.py", line 563, in run_env
    util.load_python_file(self.dir, "env.py")
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/util/pyfiles.py", line 92, in load_python_file
    module = load_module_py(module_id, path)
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/util/pyfiles.py", line 108, in load_module_py
    spec.loader.exec_module(module)  # type: ignore
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "<user_dir>alembic/env.py", line 77, in <module>
    run_migrations_online()
  File "<user_dir>alembic/env.py", line 71, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/runtime/environment.py", line 851, in run_migrations
    self.get_context().run_migrations(**kw)
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/runtime/migration.py", line 620, in run_migrations
    step.migration_fn(**kw)
  File "<user_dir>alembic/versions/f9828e772e26_init.py", line 22, in upgrade
    with op.batch_alter_table('example') as batch_op:
  File "/usr/lib/python3.10/contextlib.py", line 142, in __exit__
    next(self.gen)
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/operations/base.py", line 374, in batch_alter_table
    impl.flush()
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/operations/batch.py", line 138, in flush
    fn(*arg, **kw)
  File "<user_dir>venv/lib/python3.10/site-packages/alembic/operations/batch.py", line 469, in alter_column
    and kw["existing_type"].name  # type:ignore[attr-defined]
  File "<user_dir>venv/lib/python3.10/site-packages/sqlalchemy/sql/type_api.py", line 1462, in __getattr__
    return getattr(self.impl, key)
AttributeError: 'Text' object has no attribute 'name'

Versions.

  • OS: Archlinux
  • Python: 3.10.2
  • Alembic: 1.7.5
  • SQLAlchemy: 1.4.31
  • Database: sqlite
  • DBAPI: SQLite

Additional context
This is a regression introduced in alembic 1.7.0 with the following commit: e34f6c1

@THS-on THS-on added the requires triage New issue that requires categorization label Jan 31, 2022
@zzzeek zzzeek added batch migrations bug Something isn't working and removed requires triage New issue that requires categorization labels Jan 31, 2022
@zzzeek
Copy link
Member

zzzeek commented Jan 31, 2022

thanks for reporting will assume this is specific to batch mode

@CaselIT CaselIT added the great mcve an issue with a great mcve label Jan 31, 2022
@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the main branch:

resolve for variant before testing for schema type https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/3570

@zzzeek
Copy link
Member

zzzeek commented Feb 1, 2022

we're overdue for a release so will do that now.

@THS-on
Copy link
Author

THS-on commented Feb 1, 2022

Can confirm that this fixes the issue. Thank you for providing a fix this fast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
batch migrations bug Something isn't working great mcve an issue with a great mcve
Projects
None yet
Development

No branches or pull requests

4 participants