need to associate autogen metadata w/ operations to support metadata-bound naming conventions

[sqlalchemy-bot]

Migrated issue, originally created by Marek Baczyński (@imbaczek)

DB: SQL Server 2008R2

SQLAlchemy==0.9.3

alembic==0.6.4dev (today's master and 0.6.3 too)

def upgrade():
    op.add_column('table', sa.Column('col1', sa.Enum('one', 'two', name='col1'), nullable=True))
    op.add_column('table', sa.Column('col2', sa.Boolean(name='col2'), nullable=True))

output:

-- Running upgrade None -> 328b04eddf97

ALTER TABLE [table] ADD col1 VARCHAR(3) NULL;

GO

-- expected ck_table_col1, got col1
ALTER TABLE [table] ADD CONSTRAINT col1 CHECK (col1 IN ('one', 'two'));

GO

ALTER TABLE [table] ADD col2 BIT NULL;

GO

-- expected ck_table_col2, got col2
ALTER TABLE [table] ADD CONSTRAINT col2 CHECK (col2 IN (0, 1));

GO

env.py

# ...

from package.db import Base

target_metadata = Base.metadata

# ...

package/db.py

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"
}

metadata = MetaData(naming_convention=convention)
Base = declarative_base(metadata=metadata)

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

good catch, these constraints are generated in a different context

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• added labels: high priority

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed title from "Boolean/Enum constraint names do not honor naming " to "need to associate autogen metadata w/ operations t"

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• added labels: op directives

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• set milestone to "tier 1"

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

  Extensive changes have been made to more fully support SQLAlchemy's new
  naming conventions feature.  Note that while SQLAlchemy has added this
  feature as of 0.9.2, some additional fixes in 0.9.4 are needed to
  resolve some of the issues:

  1. The :class:`.Operations` object now takes into account the naming
     conventions that are present on the :class:`.MetaData` object that's
     associated using :paramref:`~.EnvironmentContext.configure.target_metadata`.
     When :class:`.Operations` renders a constraint directive like
     ``ADD CONSTRAINT``, it now will make use of this naming convention
     when it produces its own temporary :class:`.MetaData` object.

  2. Note however that the autogenerate feature in most cases generates
     constraints like foreign keys and unique constraints with the
     final names intact; the only exception are the constraints implicit
     with a schema-type like Boolean or Enum.  In most of these cases,
     the naming convention feature will not take effect for these constraints
     and will instead use the given name as is, with one exception....

  3. Naming conventions which use the ``"%(constraint_name)s"`` token, that
     is, produce a new name that uses the original name as a component,
     will still be pulled into the naming convention converter and be
     converted.  The problem arises when autogenerate renders a constraint
     with it's already-generated name present in the migration file's source
     code, the name will be doubled up at render time due to the combination
     of #1 and #2.  So to work around this, autogenerate now renders these
     already-tokenized names using the new :meth:`.Operations.f` component.
     This component is only generated if **SQLAlchemy 0.9.4** or greater
     is in use.

  Therefore it is highly recommended that an upgrade to Alembic 0.6.4
  be accompanied by an upgrade of SQLAlchemy 0.9.4, if the new naming
  conventions feature is used.

fixes #183

→ a6b02d1

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

OK I really had to go nuts on this one to cover the bases, and its not clear that I took the best approach. I like that an autogenerate run produces the constraints with their final name inline; this allows migration files to be independent of naming convention changes. However I balked at going as far as making it do this automatically within Boolean() and Enum(), and also, I still wanted a way to write directives in a migration script where the naming conventions do take effect. Usually, whether or not the name is None makes this easy to decide. But because i wanted this stupid "constraint_name" token (which is because that's how i was using it already at work), it's not that simple (I don't know of any other good way to give CHECK constraints a deterministic and unique name, there has to be some kind of token, unless I pull out the columns I guess, but multiple CHECK constraints on a single column is very possible).

So the new feature op.f() is now used to disambiguate names that have already been tokenized vs. those that have not. It requires SQLAlchemy 0.9.4 for it to be usable.

If you want to try out SQLAlchemy 0.9.4 from master and this, feel free to let me know that this improves things.

[sqlalchemy-bot]

Marek Baczyński (@imbaczek) wrote:

I can confirm that using both SQLAlchemy and alembic master works as advertised. Thanks!

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Fixed another bug regarding naming conventions, continuing
from 🎫183, where add_index()
drop_index() directives would not correctly render the f()
construct when the index contained a convention-driven name.
fixes #194 re: #183

→ a9fa7d9