alembic icon indicating copy to clipboard operation
alembic copied to clipboard

apply op.f() to all reflected constraint names?

Open sqlalchemy-bot opened this issue 10 years ago • 7 comments

Migrated issue, originally created by Michael Bayer (@zzzeek)

probably. these names are from the DB and should never have naming conventions applied to them.

diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py
index 2aae962..410b313 100644
--- a/alembic/autogenerate/compare.py
+++ b/alembic/autogenerate/compare.py
@@ -4,7 +4,7 @@ import logging
 from .. import compat
 from sqlalchemy.util import OrderedSet
 import re
-from .render import _user_defined_render
+from .render import _user_defined_render, conv
 import contextlib
 from alembic.ddl.base import _fk_spec
 
@@ -81,6 +81,7 @@ def _compare_tables(conn_table_names, metadata_table_names,
     existing_tables = conn_table_names.intersection(metadata_table_names)
 
     existing_metadata = sa_schema.MetaData()
+
     conn_column_info = {}
     for s, tname in existing_tables:
         name = sa_schema._get_table_key(tname, s)
@@ -125,7 +126,7 @@ def _compare_tables(conn_table_names, metadata_table_names,
 def _make_index(params, conn_table):
     # TODO: add .info such as 'duplicates_constraint'
     return sa_schema.Index(
-        params['name'],
+        conv(params['name']),
         *[conn_table.c[cname] for cname in params['column_names']],
         unique=params['unique']
     )
@@ -135,7 +136,7 @@ def _make_unique_constraint(params, conn_table):
     # TODO: add .info such as 'duplicates_index'
     return sa_schema.UniqueConstraint(
         *[conn_table.c[cname] for cname in params['column_names']],
-        name=params['name']
+        name=conv(params['name'])
     )
 
 
@@ -151,7 +152,7 @@ def _make_foreign_key(params, conn_table):
         ondelete=params.get('ondelete'),
         deferrable=params.get('deferrable'),
         initially=params.get('initially'),
-        name=params['name']
+        name=conv(params['name'])
     )
     # needed by 0.7
     conn_table.append_constraint(const)
diff --git a/alembic/autogenerate/render.py b/alembic/autogenerate/render.py
index 13830cf..7ff6f3e 100644
--- a/alembic/autogenerate/render.py
+++ b/alembic/autogenerate/render.py
@@ -18,6 +18,8 @@ try:
         else:
             return name
 except ImportError:
+    conv = lambda name: name
+
     def _render_gen_name(autogen_context, name):
         return name
 

sqlalchemy-bot avatar Jan 13 '15 22:01 sqlalchemy-bot

Michael Bayer (@zzzeek) wrote:

this is a bug, it's a big deal. One user is struggling w/ naming conventions in #452, #453, #454, but when we make use of the %(constraint_name)s token, things get crazy. it isn't impacting much because people only use that token for CHECK constraints which don't compare in autogen right now. But if we use it in a UniqueConstraint the bug is clear.

given a model thats starts at rev1 and goes to rev2:

target_metadata = metadata = MetaData(naming_convention={
        "uq": "uq_%(table_name)s_%(constraint_name)s",
    })

rev = 2

if rev == 1:
    Table('forms', metadata,
          Column('id', Integer, primary_key=True),
          Column('org_code', Text),
          Column('form_name', Text),
          UniqueConstraint('org_code', name='uu')
          )

elif rev == 2:
    Table('forms', metadata,
          Column('id', Integer, primary_key=True),
          Column('org_code', Text),
          Column('form_name', Text),
          UniqueConstraint('org_code', 'form_name', name='uu')
          )

the second migration when we switch to rev2:

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_constraint(u'uq_forms_uu', 'forms', type_='unique')
    op.create_unique_constraint(op.f('uq_forms_uu'), 'forms', ['org_code', 'form_name'])
    # ### end Alembic commands ###


def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_constraint(op.f('uq_forms_uu'), 'forms', type_='unique')
    op.create_unique_constraint(u'uq_forms_uu', 'forms', ['org_code'])
    # ### end Alembic commands ###

fails:

sqlalchemy.exc.ProgrammingError: (psycopg2.ProgrammingError) constraint "uq_forms_uq_forms_uu" of relation "forms" does not exist
 [SQL: 'ALTER TABLE forms DROP CONSTRAINT uq_forms_uq_forms_uu']

sqlalchemy-bot avatar Sep 16 '17 00:09 sqlalchemy-bot

Michael Bayer (@zzzeek) wrote:

https://gerrit.sqlalchemy.org/521

sqlalchemy-bot avatar Sep 16 '17 00:09 sqlalchemy-bot

Changes by Michael Bayer (@zzzeek):

  • edited description

sqlalchemy-bot avatar Sep 16 '17 00:09 sqlalchemy-bot

Changes by Michael Bayer (@zzzeek):

  • changed milestone from "tier 1" to "fasttrack"

sqlalchemy-bot avatar Sep 16 '17 00:09 sqlalchemy-bot

see also #1458

zzzeek avatar Apr 17 '24 17:04 zzzeek

why would we ever render a string name coming from a schema object in an op and not have op.f() around it? if the name in an op was not typed by hand, it came from a constraint, meaning, naming convention was applied. right?

zzzeek avatar Apr 17 '24 17:04 zzzeek

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

add option which will apply op.f() to all constraint names coming from autogenerate https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5265

sqla-tester avatar Apr 17 '24 18:04 sqla-tester