Constraint drops not defined in autogenerated `downgrade` function

[sqlalchemy-bot]

Migrated issue, originally created by Adamos Kyriakou (@somada141)

Hi there,

I've been scouring the googs and nets for an answer to this and by what I've seen it should work so here I am.

Let's assume the following simple model:

# coding=utf-8

import sqlalchemy
import sqlalchemy.orm
import sqlalchemy.types
from sqlalchemy.ext.declarative import declarative_base

# create declarative base
metadata = sqlalchemy.MetaData(
    naming_convention={
        "ix": "sth_ix_%(column_0_label)s",
        "uq": "sth_uq_%(table_name)s_%(column_0_name)s",
        "fk": "sth_fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
        "pk": "sth_pk_%(table_name)s"
      }
)
Base = declarative_base(metadata=metadata)

class Parent(Base):
    __tablename__ = "parents"

    parent_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        primary_key=True,
    )

    name = sqlalchemy.Column(
        sqlalchemy.types.Unicode(length=80),
    )

    age = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        index=True
    )

    __table_args__ = (
        sqlalchemy.UniqueConstraint(
            "name"
        ),
    )

class Child(Base):
    __tablename__ = "children"

    child_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        primary_key=True,
    )

    parent_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        sqlalchemy.ForeignKey("parents.parent_id")
    )

As you can see I'm using the naming_conventions feature, developing the above on alembic v0.9.6 and sqlalchemy v1.1.15, as detailed under http://alembic.zzzcomputing.com/en/latest/naming.html#integration-of-naming-conventions-into-operations-autogenerate. The silly sth prefix is only there so I can ensure the convention is being picked up by alembic.

My alembic.ini simply defined a sqlalchemy.url = sqlite:///memory while here's what my env.py looks like:

from __future__ import with_statement
from alembic import context
from sqlalchemy import engine_from_config, pool
from logging.config import fileConfig

import os
import sys

dir_current = os.path.split(os.path.abspath(__file__))[0]
dir_parent = os.path.abspath(os.path.join(dir_current, os.pardir))
sys.path.append(dir_parent)

import dal_base
import orm

config = context.config

fileConfig(config.config_file_name)

target_metadata = orm.Base.metadata

def run_migrations_offline():

    url = config.get_main_option("sqlalchemy.url")
    context.configure(
        url=url, target_metadata=target_metadata, literal_binds=True)

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

def run_migrations_online():

    connectable = engine_from_config(
        config.get_section(config.config_ini_section),
        prefix='sqlalchemy.',
        poolclass=pool.NullPool)

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

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

if context.is_offline_mode():
    run_migrations_offline()
else:
    run_migrations_online()

Upon doing a alembic revision --autogenerate I get the following revision script:

"""empty message

Revision ID: a95bae84cbe8
Revises: 
Create Date: 2017-11-27 21:00:40.103536

"""
from alembic import op
import sqlalchemy as sa

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

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.create_table('parents',
    sa.Column('parent_id', sa.Integer(), nullable=False),
    sa.Column('name', sa.Unicode(length=80), nullable=True),
    sa.Column('age', sa.Integer(), nullable=True),
    sa.PrimaryKeyConstraint('parent_id', name=op.f('sth_pk_parents')),
    sa.UniqueConstraint('name', name=op.f('sth_uq_parents_name'))
    )
    op.create_index(op.f('sth_ix_parents_age'), 'parents', ['age'], unique=False)
    op.create_table('children',
    sa.Column('child_id', sa.Integer(), nullable=False),
    sa.Column('parent_id', sa.Integer(), nullable=True),
    sa.ForeignKeyConstraint(['parent_id'], ['parents.parent_id'], name=op.f('sth_fk_children_parent_id_parents')),
    sa.PrimaryKeyConstraint('child_id', name=op.f('sth_pk_children'))
    )
    # ### end Alembic commands ###

def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_table('children')
    op.drop_index(op.f('sth_ix_parents_age'), table_name='parents')
    op.drop_table('parents')
    # ### end Alembic commands ###

So while all constraints are defined in the upgrade function with 'conventional' names none of them get explicitly dropped in the downgrade function. While in the above example there's no issues with the alembic upgrade head and alembic downgrade base calls, my actual model hits a snag with a Cannot drop index '<index-name-here>: needed in a foreign key constraint.

Is there a way to make autogenerate create the constraint drops in the downgrade function? Am I missing something?

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

I'm not familiar with the conditions which cause "Cannot drop index ': needed in a foreign key constraint", doesn't offhand sound very SQLite-esque. Feel free to provide real-world details.

You can rewrite the drop of a table in terms of the individual unique constraints and such using a migration rewriter. this is described in http://alembic.zzzcomputing.com/en/latest/api/autogenerate.html#fine-grained-autogenerate-generation-with-rewriters where you'd use a rewriter for DropTableOp to look in element.table.constraints and emit a DROP for the given names ahead of time.

Doesn't look like anything is needed in Alembic here as the directive can be rewritten to suit this occurrence, but again feel free to share details how to reproduce to provide some context.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

oh, if the issue is that you have mutually-dependent foreign keys, and one of the foreign key constraints needs to be first dropped externally, the issue at #326 marks this as an eventual feature.

[sqlalchemy-bot]

Adamos Kyriakou (@somada141) wrote:

Sorry I should've specified, the actual use-case is on MySQL, I used SQLite above just to demo the generation of the revision but the same behaviour was seen on MySQL. I will prepare a better example and update.

[sqlalchemy-bot]

Adamos Kyriakou (@somada141) wrote:

hey @zzzeek I put together a non-crappy example which I've tested on MySQL and reproduced my issue.

Consider the following amended orm.py:

# coding=utf-8

import sqlalchemy
import sqlalchemy.orm
import sqlalchemy.types
from sqlalchemy.ext.declarative import declarative_base

# create declarative base
metadata = sqlalchemy.MetaData(
    naming_convention={
        "ix": "sth_ix_%(column_0_label)s",
        "uq": "sth_uq_%(table_name)s_%(column_0_name)s",
        "fk": "sth_fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
        "pk": "sth_pk_%(table_name)s"
      }
)
Base = declarative_base(metadata=metadata)

class Parent(Base):
    __tablename__ = "parents"

    parent_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        primary_key=True,
    )

    name = sqlalchemy.Column(
        sqlalchemy.types.Unicode(length=80),
    )

    age = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        index=True
    )

    children = sqlalchemy.orm.relationship(
        argument="Child",
        secondary="parents_children",
        back_populates="parents",
        innerjoin=True,
    )

    __table_args__ = (
        sqlalchemy.UniqueConstraint(
            "name"
        ),
    )

class Child(Base):
    __tablename__ = "children"

    child_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        primary_key=True,
    )

    parent_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        sqlalchemy.ForeignKey("parents.parent_id")
    )

    parents = sqlalchemy.orm.relationship(
        argument="Parent",
        secondary="parents_children",
        back_populates="children",
    )

class ParentChild(Base):
    __tablename__ = "parents_children"

    parent_child_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        primary_key=True,
    )

    parent_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        sqlalchemy.ForeignKey("parents.parent_id"),
        index=True,
    )

    child_id = sqlalchemy.Column(
        sqlalchemy.types.Integer(),
        sqlalchemy.ForeignKey("children.child_id"),
        index=True,
    )

    __table_args__ = (
        sqlalchemy.UniqueConstraint(
            "parent_id",
            "child_id",
        ),
    )

The above is much closer to the real-world ORM I'm dealing with as in it uses associative join tables. In the above example we forge a many-to-many between parents and children with relationships defined on both sides.

There were no changes to env.py while the alembic.ini was merely changed to point to the MySQL server as such:

sqlalchemy.url = mysql+mysqldb://<user>:<pass>@localhost:3306/rubbish?charset=utf8mb4

Upon running alembic revision --autogenerate I get the following migration script:

"""empty message

Revision ID: 3d703c0e98b1
Revises: 
Create Date: 2017-11-28 10:26:14.803349

"""
from alembic import op
import sqlalchemy as sa

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

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.create_table('parents',
    sa.Column('parent_id', sa.Integer(), nullable=False),
    sa.Column('name', sa.Unicode(length=80), nullable=True),
    sa.Column('age', sa.Integer(), nullable=True),
    sa.PrimaryKeyConstraint('parent_id', name=op.f('sth_pk_parents')),
    sa.UniqueConstraint('name', name=op.f('sth_uq_parents_name'))
    )
    op.create_index(op.f('sth_ix_parents_age'), 'parents', ['age'], unique=False)
    op.create_table('children',
    sa.Column('child_id', sa.Integer(), nullable=False),
    sa.Column('parent_id', sa.Integer(), nullable=True),
    sa.ForeignKeyConstraint(['parent_id'], ['parents.parent_id'], name=op.f('sth_fk_children_parent_id_parents')),
    sa.PrimaryKeyConstraint('child_id', name=op.f('sth_pk_children'))
    )
    op.create_table('parents_children',
    sa.Column('parent_child_id', sa.Integer(), nullable=False),
    sa.Column('parent_id', sa.Integer(), nullable=True),
    sa.Column('child_id', sa.Integer(), nullable=True),
    sa.ForeignKeyConstraint(['child_id'], ['children.child_id'], name=op.f('sth_fk_parents_children_child_id_children')),
    sa.ForeignKeyConstraint(['parent_id'], ['parents.parent_id'], name=op.f('sth_fk_parents_children_parent_id_parents')),
    sa.PrimaryKeyConstraint('parent_child_id', name=op.f('sth_pk_parents_children')),
    sa.UniqueConstraint('parent_id', 'child_id', name=op.f('sth_uq_parents_children_parent_id'))
    )
    op.create_index(op.f('sth_ix_parents_children_child_id'), 'parents_children', ['child_id'], unique=False)
    op.create_index(op.f('sth_ix_parents_children_parent_id'), 'parents_children', ['parent_id'], unique=False)
    # ### end Alembic commands ###

def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_index(op.f('sth_ix_parents_children_parent_id'), table_name='parents_children')
    op.drop_index(op.f('sth_ix_parents_children_child_id'), table_name='parents_children')
    op.drop_table('parents_children')
    op.drop_table('children')
    op.drop_index(op.f('sth_ix_parents_age'), table_name='parents')
    op.drop_table('parents')
    # ### end Alembic commands ###

where once again there's no explicit drops for the constraints albeit named properly.

Now while alembic upgrade head works fine doing a alembic downgrade base throws the following error:

sqlalchemy.exc.OperationalError: (_mysql_exceptions.OperationalError) (1553, "Cannot drop index 'sth_ix_parents_children_child_id': needed in a foreign key constraint") [SQL: u'\nDROP INDEX sth_ix_parents_children_child_id ON parents_children']

with the full traceback being:

INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running downgrade 3d703c0e98b1 -> , empty message
Traceback (most recent call last):
  File "/Users/adam/anaconda2/envs/py27/bin/alembic", line 11, in <module>
    sys.exit(main())
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/config.py", line 479, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/config.py", line 473, in main
    self.run_cmd(cfg, options)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/config.py", line 456, in run_cmd
    **dict((k, getattr(options, k, None)) for k in kwarg)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/command.py", line 294, in downgrade
    script.run_env()
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/script/base.py", line 425, in run_env
    util.load_python_file(self.dir, 'env.py')
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/util/pyfiles.py", line 81, in load_python_file
    module = load_module_py(module_id, path)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/util/compat.py", line 141, in load_module_py
    mod = imp.load_source(module_id, path, fp)
  File "alembic/env.py", line 51, in <module>
    run_migrations_online()
  File "alembic/env.py", line 46, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/runtime/environment.py", line 836, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/runtime/migration.py", line 330, in run_migrations
    step.migration_fn(**kw)
  File "/Users/adam/Downloads/sqlb/alembic/versions/3d703c0e98b1_.py", line 52, in downgrade
    op.drop_index(op.f('sth_ix_parents_children_child_id'), table_name='parents_children')
  File "<string>", line 8, in drop_index
  File "<string>", line 3, in drop_index
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/operations/ops.py", line 964, in drop_index
    return operations.invoke(op)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/operations/base.py", line 319, in invoke
    return fn(self, operation)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/operations/toimpl.py", line 94, in drop_index
    operation.to_index(operations.migration_context)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/ddl/impl.py", line 209, in drop_index
    self._exec(schema.DropIndex(index))
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/alembic/ddl/impl.py", line 118, in _exec
    return conn.execute(construct, *multiparams, **params)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 945, in execute
    return meth(self, multiparams, params)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/sqlalchemy/sql/ddl.py", line 68, in _execute_on_connection
    return connection._execute_ddl(self, multiparams, params)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1002, in _execute_ddl
    compiled
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1189, in _execute_context
    context)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1402, in _handle_dbapi_exception
    exc_info
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/sqlalchemy/util/compat.py", line 203, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb, cause=cause)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1182, in _execute_context
    context)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/sqlalchemy/engine/default.py", line 470, in do_execute
    cursor.execute(statement, parameters)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/MySQLdb/cursors.py", line 205, in execute
    self.errorhandler(self, exc, value)
  File "/Users/adam/anaconda2/envs/py27/lib/python2.7/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
sqlalchemy.exc.OperationalError: (_mysql_exceptions.OperationalError) (1553, "Cannot drop index 'sth_ix_parents_children_child_id': needed in a foreign key constraint") [SQL: u'\nDROP INDEX sth_ix_parents_children_child_id ON parents_children']

Before I invest into writing rewriters can you verify that the behaviour seen above is expected/known? Does the above qualify as a mutually-dependent foreign key?

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

So first fun fact, you do not need these indexes at all (assuming you are using InnoDB which you should be):

https://dev.mysql.com/doc/refman/5.7/en/create-table-foreign-keys.html

MySQL requires indexes on foreign keys and referenced keys so that foreign key checks can be fast and not require a table scan. In the referencing table, there must be an index where the foreign key columns are listed as the first columns in the same order. Such an index is created on the referencing table automatically if it does not exist. This index might be silently dropped later, if you create another index that can be used to enforce the foreign key constraint. index_name, if given, is used as described previously.

these indexes are created and dropped automatically by MySQL.

Beyond that, what's happening there is that it's emitting DROP INDEX for two indexes that are part of the table to be dropped in any case. So you would only need to remove those two "drop_index" calls because the table is being dropped anyway.

I don't want to make this an automatic behavior of Alembic since it is surprising. However, I do encourage that we have a cookbook recipe that shows off a rewriter such as this one. that will be in the next comment.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Add cookbook recipe for don't render DROP INDEX

Filters out DropIndexOp when there is a corresponding DropTableOp.

Change-Id: I7baadf6e5b9f669c875aeeaccefb19cb5e105953 Fixes: #467

→ 4f7f973ec1dd

[sqlalchemy-bot]

Adamos Kyriakou (@somada141) wrote:

Thanks @zzzeek. While the above makes sense for InnoDB I had to design the ORM with an eventual migration to Postgres in mind where AFAIK there's no indexes being created for foreign keys by default (hence the parameter).

Thus I think that a custom rewriter may be my only option here.

One last thing: do I understand correctly that alembic won't automatically create drops for constraints? Other issues and stackoverflow questions suggested that it would.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

it will create drops for constraints if that is the thing that is detected as being what changed. if the whole table is being affected, then it won't generate for constraints. this is because the "downgrade()" section is rendered as a mirror image of the "upgrade()" section. for the creation of a table, the constraints are inline, so the "mirror" is a drop table. for the creation of a table plus creation of some indexes on it, the "mirror" drop indexes then drop the table.

given that behavior (because I'm looking at the code), it's possible there's an inconsistency going the other way. if an upgrade() section includes that an existing table is dropped, i think it might not be rendering the drop index directives separately.

so...there's a bug there! I added #468. it's skipping the indexes entirely.

[sqlalchemy-bot]

Adamos Kyriakou (@somada141) wrote:

cool! glad something came out of this :). Thanks @zzzeek !

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed