sqlalchemy-bot / test_alembic_1

0 stars 0 forks source link

Autogen, when adding a foreign key does not create a name for it - downgrade is therefore broken. #500

Closed sqlalchemy-bot closed 6 years ago

sqlalchemy-bot commented 6 years ago

Migrated issue, originally created by Jack Grahl (jwg4)

When adding a foreign key constraint to two pre-existing tables, thus going from:

#!python

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

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)

class B(Base):
    __tablename__ = 'b'
    id = Column(Integer, primary_key=True)
    another_int = Column(Integer),
        nullable=False
    )

to

#!python

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

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)

class B(Base):
    __tablename__ = 'b'
    id = Column(Integer, primary_key=True)
    another_int = Column(
        ForeignKey(
            'a.id',
            ondelete='CASCADE',
            deferrable=True,
            initially='DEFERRED',
        ),
        nullable=False
    )

autogenerates a migration where the FK constraint is not given any name like this (full output of revision --autogenerate):

#!python
from alembic import op
import sqlalchemy as sa

revision = '4968165e5bde'
down_revision = '76f3885cd6b8'
branch_labels = None
depends_on = None

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.alter_column('b', 'another_int',
               existing_type=sa.INTEGER(),
               nullable=False)
    op.create_foreign_key(None, 'b', 'a', ['another_int'], ['id'], ondelete='CASCADE', initially='DEFERRED', deferrable=True)
    # ### end Alembic commands ###

def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_constraint(None, 'b', type_='foreignkey')
    op.alter_column('b', 'another_int',
               existing_type=sa.INTEGER(),
               nullable=True)
    # ### end Alembic commands ###

As can be seen, create_foreign_key is called with a name argument of None. This means that the name is left to the default. However, that means that without any warning, the upgrade script will succeed but that the downgrade script will then fail (with postgresql and psycopg2), because there is no name for the foreign key that should be removed.

(venv) jack@neptune-vm:~/z/alembic_test$ python -m alembic.config downgrade 76f3885
INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO  [alembic.runtime.migration] Will assume transactional DDL.
INFO  [alembic.runtime.migration] Running downgrade 4968165e5bde -> 76f3885cd6b8, empty message
Traceback (most recent call last):
  File "/usr/lib/python3.5/runpy.py", line 184, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.5/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/config.py", line 489, in <module>
    main()
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/config.py", line 486, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/config.py", line 480, in main
    self.run_cmd(cfg, options)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/config.py", line 463, in run_cmd
    **dict((k, getattr(options, k, None)) for k in kwarg)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/command.py", line 294, in downgrade
    script.run_env()
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/script/base.py", line 427, in run_env
    util.load_python_file(self.dir, 'env.py')
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/util/pyfiles.py", line 81, in load_python_file
    module = load_module_py(module_id, path)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/util/compat.py", line 83, in load_module_py
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 665, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "alembic/env.py", line 72, in <module>
    run_migrations_online()
  File "alembic/env.py", line 67, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/runtime/environment.py", line 836, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/runtime/migration.py", line 330, in run_migrations
    step.migration_fn(**kw)
  File "/home/jack/z/alembic_test/alembic/versions/4968165e5bde_.py", line 30, in downgrade
    op.drop_constraint(None, 'b', type_='foreignkey')
  File "<string>", line 8, in drop_constraint
  File "<string>", line 3, in drop_constraint
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/operations/ops.py", line 148, in drop_constraint
    return operations.invoke(op)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/operations/base.py", line 319, in invoke
    return fn(self, operation)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/operations/toimpl.py", line 146, in drop_constraint
    schema=operation.schema,
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/ddl/impl.py", line 183, in drop_constraint
    self._exec(schema.DropConstraint(const))
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/alembic/ddl/impl.py", line 118, in _exec
    return conn.execute(construct, *multiparams, **params)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/engine/base.py", line 948, in execute
    return meth(self, multiparams, params)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/sql/ddl.py", line 68, in _execute_on_connection
    return connection._execute_ddl(self, multiparams, params)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/engine/base.py", line 1003, in _execute_ddl
    if not self.schema_for_object.is_default else None)
  File "<string>", line 1, in <lambda>
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/sql/elements.py", line 442, in compile
    return self._compiler(dialect, bind=bind, **kw)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/sql/ddl.py", line 26, in _compiler
    return dialect.ddl_compiler(dialect, self, **kw)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/sql/compiler.py", line 219, in __init__
    self.string = self.process(self.statement, **compile_kwargs)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/sql/compiler.py", line 245, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/ext/compiler.py", line 435, in <lambda>
    lambda *arg, **kw: existing(*arg, **kw))
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/ext/compiler.py", line 474, in __call__
    return fn(element, compiler, **kw)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/ext/compiler.py", line 426, in _wrap_existing_dispatch
    return existing_dispatch(element, compiler, **kw)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/sql/visitors.py", line 81, in _compiler_dispatch
    return meth(self, **kw)
  File "/home/jack/z/alembic_test/venv/lib/python3.5/site-packages/sqlalchemy/sql/compiler.py", line 2665, in visit_drop_constraint
    "it has no name" % drop.element)
sqlalchemy.exc.CompileError: Can't emit DROP CONSTRAINT for constraint ForeignKeyConstraint(<sqlalchemy.sql.base.ColumnCollection object at 0x7f4e5e1c9710>, None, table=Table('b', MetaData(bind=None), schema=None)); it has no name

Desired behavior here would be that alembic create a name for the FK constraint and use it as an argument in both create_foreign_key and in drop_constraint so that both upgrade and downgrade work. Failing this, it would be good to have a warning, for example emitted as a comment in the generated code, so that it was clear that a name for the constraint should be chosen by the user and passed to both functions.

Thanks a lot for all your hard work on alembic!

sqlalchemy-bot commented 6 years ago

Jack Grahl (jwg4) wrote:

Hi, I have found your discussion of this in the docs. This solves my problem, thanks, and sorry I didn't see this earlier.

The link for anyone else who has the same issue: http://alembic.zzzcomputing.com/en/latest/naming.html

sqlalchemy-bot commented 6 years ago

Jack Grahl (jwg4) wrote:

This is dealt with in the documentation and solved by setting a naming convention for constraints. http://alembic.zzzcomputing.com/en/latest/naming.html

sqlalchemy-bot commented 6 years ago

Michael Bayer (zzzeek) wrote:

yep. sorry sqlalchemy/alembic doesn't like to make assumptions :)