New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autogen: bogus drop index/create constraint in SQL Server 2008R2 #185

Closed
sqlalchemy-bot opened this Issue Mar 12, 2014 · 11 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Mar 12, 2014

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

DB: SQL Server 2008R2

SQLAlchemy==0.9.3

alembic==0.6.4dev

from sqlalchemy import Column, Integer, String, MetaData, ForeignKey
from sqlalchemy import Enum, Text, Sequence, BigInteger, DateTime, Boolean
from sqlalchemy import UniqueConstraint, CheckConstraint
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import relationship

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)

class Table(Base):
    __tablename__ = 'table'
    id = Column(Integer, primary_key=True)
    col1 = Column(Enum('one', 'two', name='col1'), nullable=True)
    col2 = Column(Boolean(name='col2'), nullable=True)

    __table_args__ = (
        UniqueConstraint('col1', 'col2'),
    )

issuing metadata.create_all() and right after that (without modifying the model) alembic revision -m "..." --autogenerate results in a surprising migration:

def upgrade():
    ### commands auto generated by Alembic - please adjust! ###
    op.drop_index('uq_table_col1', table_name='table')
    op.create_unique_constraint('uq_table_col1', 'table', ['col1', 'col2'])
    ### end Alembic commands ###


def downgrade():
    ### commands auto generated by Alembic - please adjust! ###
    op.drop_constraint('uq_table_col1', 'table')
    op.create_index('uq_table_col1', 'table', ['col1', 'col2'], unique=True)
    ### end Alembic commands ###

expected result would be a noop migration.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 12, 2014

Michael Bayer (@zzzeek) wrote:

its unusual I've not observed this as I use SQL server quite a bit but this would indicate the reflection process is getting indexes and unique constraints mixed up. we have the same issue with PG and MySQL, in different ways, and the logic goes through huge hoops to try to be conservative on that so I'm puzzled what new combination SQL Server is reporting.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 12, 2014

Changes by Michael Bayer (@zzzeek):

  • added labels: high priority
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 12, 2014

Changes by Michael Bayer (@zzzeek):

  • added labels: autogenerate - detection
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 12, 2014

Changes by Michael Bayer (@zzzeek):

  • set milestone to "tier 1"
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 12, 2014

Marek Baczyński (@imbaczek) wrote:

what kind of information would help you and how to get it?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 12, 2014

Michael Bayer (@zzzeek) wrote:

I probably need to fire up my SQL server here and just run what you have

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 13, 2014

Michael Bayer (@zzzeek) wrote:

OK this is pretty simple. mssql dialect isn't doing unique constraints yet.

(Pdb) insp.get_unique_constraints('table')
*** NotImplementedError: 
(Pdb) insp.get_indexes('table')
INFO  [sqlalchemy.engine.base.Engine] select ind.index_id, ind.is_unique, ind.name from sys.indexes as ind join sys.tables as tab on ind.object_id=tab.object_id join sys.schemas as sch on sch.schema_id=tab.schema_id where tab.name = ? and sch.name=? and ind.is_primary_key=0
INFO  [sqlalchemy.engine.base.Engine] ('table', 'dbo')
INFO  [sqlalchemy.engine.base.Engine] select ind_col.index_id, ind_col.object_id, col.name from sys.columns as col join sys.tables as tab on tab.object_id=col.object_id join sys.index_columns as ind_col on (ind_col.column_id=col.column_id and ind_col.object_id=tab.object_id) join sys.schemas as sch on sch.schema_id=tab.schema_id where tab.name=? and sch.name=?
INFO  [sqlalchemy.engine.base.Engine] ('table', 'dbo')
[{'unique': True, 'name': u'uq_table_col1', 'column_names': [u'col1', u'col2']}]

neither is oracle. but mssql is doing that "we report unique constraints as indexes" thing. So let's try to be smoother about that... perhaps if we detect an index/uq switch but we know unique index reflection isn't working, that's the sign.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 13, 2014

Michael Bayer (@zzzeek) wrote:

The feature that keeps on giving, index/unique constraint autogenerate
detection, has even more fixes, this time to accommodate database dialects
that both don't yet report on unique constraints, but the backend
does report unique constraints as indexes. The logic
Alembic uses to distinguish between "this is an index!" vs.
"this is a unique constraint that is also reported as an index!" has now
been further enhanced to not produce unwanted migrations when the dialect
is observed to not yet implement get_unique_constraints() (e.g. mssql).
Note that such a backend will no longer report index drops for unique
indexes, as these cannot be distinguished from an unreported unique
index. fixes #185

4bb4d50

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 13, 2014

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Mar 14, 2014

Marek Baczyński (@imbaczek) wrote:

fix confirmed, thanks!

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 3, 2014

Changes by Michael Bayer (@zzzeek):

  • removed milestone (was: "tier 1")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment