change to no longer sort index cols for index sig produces false positives on mysql

[provinzio]

Describe the bug
Although my database with unique constraints across multiple columns matches my table model exactly, alembic generates drop_index/drop_constraint commands.

Expected behavior
alembic revision --autogenerate shouldn't generate any code

To Reproduce

# This is an example table of mine with a unique constraint across multiple columns
class DisturbanceRecorder(Base):
    __tablename__ = 'disturbance_recorders'
    __table_args__ = (sa.UniqueConstraint("device_name", "contact_id"),)

    id = sa.Column(sa.Integer, primary_key=True)
    device_name = sa.Column(sa.String(255), nullable=False)
    contact_id = sa.Column(sa.ForeignKey('contacts.id', ondelete='RESTRICT'), index=True, nullable=False)
    contact = sa.orm.relationship("Contact")

# my database has everything setup according to the table model, but
# alembic revision --autogenerate generates following code
def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_constraint('device_name', 'disturbance_recorders', type_='unique')
    op.drop_index('device_name', table_name='disturbance_recorders')
    # ### end Alembic commands ###

Versions.

• OS: Windows 10 Pro 22H2
• Python: 3.9.1
• Alembic: >= 1.10.0
• SQLAlchemy: 1.3.23
• Database: MySQL 5.5.46
• DBAPI:

Additional context
I recently updated from alembic==1.5.4 to the latest release 1.10.4 and observed the bug. I tested all versions and realized, that all version >= 1.10.0 do not work for me. I downgraded to alembic==1.9.4 which does not show any misbehavior.

Have a nice day!

[CaselIT]

Hi,

Thanks for reporting

[CaselIT]

I can't reproduce using sqlalchemy v2 or 1.4 on mysql 5.6.

Can you please post a full reproduction case?

[provinzio]

Sqlalchemy 1.3 and 1.4 differ here and there. Can you reproduce it with SQLAlchemy==1.3.23.

Reproducing the error will take me some time, as I haven't any "playground" environment, just development and production of my system (which is too complex for this).

[CaselIT]

Continue a not beeing able to reproduce on 1.3,1.4 or 2.0 and mysql 5.5, 5.6, 8

The table I'm using are:

import sqlalchemy as sa
from sqlalchemy import orm

target_metadata = sa.MetaData()

try:
    Base = orm.declarative_base(metadata=target_metadata)
except AttributeError:
    from sqlalchemy.ext.declarative import declarative_base

    Base = declarative_base(metadata=target_metadata)


class Contract(Base):
    __tablename__ = "contacts"
    id = sa.Column(sa.Integer, primary_key=True)


class DisturbanceRecorder(Base):
    __tablename__ = "disturbance_recorders"
    __table_args__ = (sa.UniqueConstraint("device_name", "contact_id"),)

    id = sa.Column(sa.Integer, primary_key=True)
    device_name = sa.Column(sa.String(25), nullable=False)
    contact_id = sa.Column(
        sa.ForeignKey("contacts.id", ondelete="RESTRICT"),
        index=True,
        nullable=False,
    )
    contact = orm.relationship("Contact")

First migration is

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.create_table(
        "contacts",
        sa.Column("id", sa.Integer(), nullable=False),
        sa.PrimaryKeyConstraint("id"),
    )
    op.create_table(
        "disturbance_recorders",
        sa.Column("id", sa.Integer(), nullable=False),
        sa.Column("device_name", sa.String(length=25), nullable=False),
        sa.Column("contact_id", sa.Integer(), nullable=False),
        sa.ForeignKeyConstraint(
            ["contact_id"], ["contacts.id"], ondelete="RESTRICT"
        ),
        sa.PrimaryKeyConstraint("id"),
        sa.UniqueConstraint("device_name", "contact_id"),
    )
    op.create_index(
        op.f("ix_disturbance_recorders_contact_id"),
        "disturbance_recorders",
        ["contact_id"],
        unique=False,
    )
    # ### end Alembic commands ###


def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_index(
        op.f("ix_disturbance_recorders_contact_id"),
        table_name="disturbance_recorders",
    )
    op.drop_table("disturbance_recorders")
    op.drop_table("contacts")
    # ### end Alembic commands ###

second migration is

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    pass
    # ### end Alembic commands ###


def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    pass
    # ### end Alembic commands ###

[zzzeek]

any chance your MySQL 5.5 is using MyISAM which maybe is not reporting on constraints?

[johaven]

Since version 10.x it seems that indeed there are constraint detection problems.

__tablename__ = 'shares_roots'
__table_args__ = (db.UniqueConstraint('sid', 'label'),
                  db.UniqueConstraint('sid', 'fid'),
                  Base.sensitive)

On my existing schema:

UNIQUE KEY `sid` (`sid`,`label`),
UNIQUE KEY `sid_2` (`sid`,`fid`)

Alembic 1.10.x:

INFO  [alembic.autogenerate.compare] Detected removed unique constraint 'sid_2' on 'shares_roots'
INFO  [alembic.autogenerate.compare] Detected removed index 'sid_2' on 'shares_roots'
  Generating /dev/migrations/versions/3218d628b90c_.py ...  done

Traceback (most recent call last):
  File "/dev/venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1905, in _execute_context
    self.dialect.do_execute(
  File "/dev/venv/lib/python3.9/site-packages/sqlalchemy/engine/default.py", line 736, in do_execute
    cursor.execute(statement, parameters)
  File "/dev/venv/lib/python3.9/site-packages/pymysql/cursors.py", line 158, in execute
    result = self._query(query)
  File "/dev/venv/lib/python3.9/site-packages/pymysql/cursors.py", line 325, in _query
    conn.query(q)
  File "/dev/venv/lib/python3.9/site-packages/pymysql/connections.py", line 549, in query
    self._affected_rows = self._read_query_result(unbuffered=unbuffered)
  File "/dev/venv/lib/python3.9/site-packages/pymysql/connections.py", line 779, in _read_query_result
    result.read()
  File "/dev/venv/lib/python3.9/site-packages/pymysql/connections.py", line 1157, in read
    first_packet = self.connection._read_packet()
  File "/dev/venv/lib/python3.9/site-packages/pymysql/connections.py", line 729, in _read_packet
    packet.raise_for_error()
  File "/dev/venv/lib/python3.9/site-packages/pymysql/protocol.py", line 221, in raise_for_error
    err.raise_mysql_exception(self._data)
  File "/dev/venv/lib/python3.9/site-packages/pymysql/err.py", line 143, in raise_mysql_exception
    raise errorclass(errno, errval)
pymysql.err.OperationalError: (1091, "Can't DROP INDEX `sid_2`; check that it exists")

Alembic 1.9.4:

INFO  [alembic.autogenerate.compare] Detected added unique constraint 'None' on '['sid', 'fid']'
  Generating /dev/migrations/versions/2f38d2c63c07_.py ...  done

INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 2f38d2c63c07, empty message

In the last case everything works

[CaselIT]

Reflection is not on almebic, but on sqlalchemy so the information should not be changing by changing the version of alembic.

I don't think the above information is sufficient for a reproduction.

[johaven]

Migrations were started with the same sqlalchemy version: 1.4.48 (and a few seconds apart)

[CaselIT]

I understand that.

Can you please what sqlalchemy reflected?

conn = getConnection()
inspector = sa.inspect(conn)
print('unique_constraints', inspector.get_unique_constraints('shares_roots'))
print('indexes', inspector.get_indexes('shares_roots'))

Without knowing the schema it seems it's not reproducible, as mentioned above

[zzzeek]

also platform, case sensitivity settings, please include a complete "SHOW CREATE TABLE"

[zzzeek]

@CaselIT I'm able to reproduce on Mariadb 10 with this model on linux:

class MyTable(Base):
    __tablename__ = 'shares_roots'

    id = Column(Integer, primary_key=True)
    sid = Column(Integer, nullable=False)
    label = Column(String(10), nullable=False)
    fid = Column(Integer, nullable=False)

    __table_args__ = (UniqueConstraint('sid', 'label'),
                  UniqueConstraint('sid', 'fid'))

also it might not happen every time, run it ia few times. does that reproduce for you?

[zzzeek]

the bad commit is 5c71fd1 not surprisingly where index generation was modified

[zzzeek]

this is the bug, so, need to understand this part, and we might need to make this "dont sort" dialect specific

diff --git a/alembic/ddl/impl.py b/alembic/ddl/impl.py
index 84f5d86..336dcbe 100644
--- a/alembic/ddl/impl.py
+++ b/alembic/ddl/impl.py
@@ -667,7 +667,7 @@ class DefaultImpl(metaclass=ImplMeta):
 
     def create_index_sig(self, index: Index) -> Tuple[Any, ...]:
         # order of col matters in an index
-        return tuple(col.name for col in index.columns)
+        return tuple(sorted(col.name for col in index.columns))
 
     def _skip_functional_indexes(self, metadata_indexes, conn_indexes):
         conn_indexes_by_name = {c.name: c for c in conn_indexes}

[zzzeek]

mysql mixes up uniques and indexes so if we no longer sort col names in the index sig, we have to unsort in unique also

iff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py
index 595631c..d01a2d3 100644
--- a/alembic/autogenerate/compare.py
+++ b/alembic/autogenerate/compare.py
@@ -452,7 +452,7 @@ class _uq_constraint_sig(_constraint_sig):
     def __init__(self, const: UniqueConstraint) -> None:
         self.const = const
         self.name = const.name
-        self.sig = tuple(sorted([col.name for col in const.columns]))
+        self.sig = tuple(col.name for col in const.columns)
 
     @property
     def column_names(self) -> List[str]:

[CaselIT]

ok, interesting. I guess we need to check if the index is an unique constraint or not and only sort if it's an index

[sqla-tester]

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

dont compare unique constraint and index sigs to each other https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4611