naming convention tries to fire off during reflection for unnamed CHECK constraint (or anything? this is reflection, naming should not be in play)

[Dave McDaniel]

After today's update to 1.1 I am now getting a migration error with the following traceback:

  File "/home/dave/Code/edgar/alembic/versions/c3337510cab5_added_signing_table.py", line 20, in upgrade
    globals()["upgrade_%s" % engine_name]()
  File "/home/dave/Code/edgar/alembic/versions/c3337510cab5_added_signing_table.py", line 51, in upgrade_main_db
    ['signing_id'], ['id'])
  File "/usr/lib/python3.4/contextlib.py", line 66, in __exit__
    next(self.gen)
  File "/usr/lib/python3.4/site-packages/alembic/operations/base.py", line 299, in batch_alter_table
    impl.flush()
  File "/usr/lib/python3.4/site-packages/alembic/operations/batch.py", line 73, in flush
    *self.reflect_args, **self.reflect_kwargs)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 436, in __new__
    metadata._remove_table(name, schema)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/util/langhelpers.py", line 60, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/util/compat.py", line 186, in reraise
    raise value
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 431, in __new__
    table._init(name, metadata, *args, **kw)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 507, in _init
    self._autoload(metadata, autoload_with, include_columns)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 519, in _autoload
    self, include_columns, exclude_columns
  File "/usr/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 1528, in run_callable
    return callable_(self, *args, **kwargs)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/engine/default.py", line 364, in reflecttable
    return insp.reflecttable(table, include_columns, exclude_columns)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/engine/reflection.py", line 605, in reflecttable
    exclude_columns, reflection_options)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/engine/reflection.py", line 727, in _reflect_fk
    **reflection_options
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 436, in __new__
    metadata._remove_table(name, schema)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/util/langhelpers.py", line 60, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/util/compat.py", line 186, in reraise
    raise value
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 431, in __new__
    table._init(name, metadata, *args, **kw)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 507, in _init
    self._autoload(metadata, autoload_with, include_columns)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 519, in _autoload
    self, include_columns, exclude_columns
  File "/usr/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 1528, in run_callable
    return callable_(self, *args, **kwargs)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/engine/default.py", line 364, in reflecttable
    return insp.reflecttable(table, include_columns, exclude_columns)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/engine/reflection.py", line 617, in reflecttable
    include_columns, exclude_columns, reflection_options)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/engine/reflection.py", line 833, in _reflect_check_constraints
    sa_schema.CheckConstraint(**const_d))
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/schema.py", line 688, in append_constraint
    constraint._set_parent_with_dispatch(self)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/base.py", line 434, in _set_parent_with_dispatch
    self.dispatch.after_parent_attach(self, parent)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/event/attr.py", line 218, in __call__
    fn(*args, **kw)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/naming.py", line 144, in _constraint_name
    newname = _constraint_name_for_table(const, table)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/naming.py", line 124, in _constraint_name_for_table
    metadata.naming_convention)
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/naming.py", line 79, in __getitem__
    return getattr(self, '_key_%s' % key)()
  File "/usr/lib/python3.4/site-packages/sqlalchemy/sql/naming.py", line 43, in _key_constraint_name
    "Naming convention including "
sqlalchemy.exc.InvalidRequestError: Naming convention including %(constraint_name)s token requires that constraint is explicitly named.

The odd thing is that the name is explicitly set in the create_foreign_key call (line 51 as identified in the traceback), which is below:

    with op.batch_alter_table('experiment', naming_convention=convention,
                              schema=None) as batch_op:
        batch_op.add_column(sa.Column('signing_id', sa.Integer(), nullable=True))
        batch_op.create_foreign_key(op.f('fk_experiment_signing_id_signing'),
                                    'signing',
                                    ['signing_id'], ['id'])

convention is defined as:

convention = {
    "ix": 'ix_%(column_0_label)s',
    "uq": "uq_%(table_name)s_%(column_0_name)s",
    "ck": "ck_%(table_name)s_%(constraint_name)s",
    "fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
    "pk": "pk_%(table_name)s"
}

Based on alembic docs, the first postional argument to create_foreign_key is the constraint_name, so I don't understand the traceback message that it is not being defined, as it is in the call via that first positional argument.

[Mike Bayer]

this has nothing to do with your foreign key. CHECK constraints are now reflected and SQLite is nice enough to support CHECK constraints that have no names at all, and the event is triggering trying to give it a name.

[Mike Bayer]

as always, this is not a regression at all, just another bug that's been there for years never noticed. this test will fail on 1.0.13 because of the same thing happening for the foreign key:

from sqlalchemy import *

m = MetaData()

q = Table(
    'q', m, Column('id', Integer, primary_key=True)
)
t = Table(
    't', m,
    Column('id', Integer, ForeignKey('q.id')),
    Column('x', Boolean())
)

e = create_engine("sqlite://", echo=True)

m.create_all(e)


m2 = MetaData(naming_convention={
    "ck": "%(constraint_name)s", "fk": "%(constraint_name)s"})
t2 = Table('t', m2, autoload_with=e)

if you're dealing with SQLite I'd take out that "ck" rule for now, you have unnamed check constraints in any case in your database.

[Mike Bayer]

OK no, this is actually not a bug for what you are doing. and in fact, if I do change this behavior, Alembic will break.

Short answer is you need to take "ck" out of your naming convention when you pass it to batch_alter_table. The naming convention passed at this level has no other function than to give unnamed constraints a name. It therefore makes no sense to use the "constraint_name" token.

I'm going to make a new issue regarding the fact that reflection on the SQLAlchemy side should not be doing what it's doing, but a new version of Alembic would need to be produced in order to be compatible with it, so that will be targeted at 1.2.

[Mike Bayer]

currently, this is the expected behavior that Alembic relies upon.

[Mike Bayer]

issue #3818 tracks if we can make this more configurable in 1.2.

[Dave McDaniel]

Understood. Thanks!

Since name looks like a positional argument in the docs I assumed it was required, regardless of passing convention into the batch context manager.

Also this occured on the same commit that had previously passed in a CI build, so it was clearly linked to the update on the cheeseshop--I should have added that context, sorry if that caused you more time.

Thanks for this incredible library! As well as alembic!

[Mike Bayer]

I knew immediately it was the CHECK constraint reflection, adding a new kind of reflection always breaks things in alembic. I was just surprised that the reflection process still applies naming conventions and then worse that at some point I threw a feature into batch that relies upon it.