From 2108076362cc76130ae61e14c7a0120a1f0a59c8 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 20 Feb 2015 19:20:53 -0500 Subject: - Fixed bug where MySQL backend would report dropped unique indexes and/or constraints as both at the same time. This is because MySQL doesn't actually have a "unique constraint" construct that reports differently than a "unique index", so it is present in both lists. The net effect though is that the MySQL backend will report a dropped unique index/constraint as an index in cases where the object was first created as a unique constraint, if no other information is available to make the decision. This differs from other backends like Postgresql which can report on unique constraints and unique indexes separately. fixes #276 --- alembic/ddl/mysql.py | 34 ++++++++++++++++++++++++++++++++++ docs/build/changelog.rst | 15 +++++++++++++++ tests/requirements.py | 4 ++++ tests/test_autogen_indexes.py | 29 ++++++++++++++++++++++------- 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/alembic/ddl/mysql.py b/alembic/ddl/mysql.py index 8be4543..7956185 100644 --- a/alembic/ddl/mysql.py +++ b/alembic/ddl/mysql.py @@ -9,6 +9,7 @@ from .base import ColumnNullable, ColumnName, ColumnDefault, \ ColumnType, AlterColumn, format_column_name, \ format_server_default from .base import alter_table +from ..autogenerate import compare class MySQLImpl(DefaultImpl): @@ -123,6 +124,39 @@ class MySQLImpl(DefaultImpl): if idx.name in removed: metadata_indexes.remove(idx) + # then dedupe unique indexes vs. constraints, since MySQL + # doesn't really have unique constraints as a separate construct. + # but look in the metadata and try to maintain constructs + # that already seem to be defined one way or the other + # on that side. See #276 + metadata_uq_names = set([ + cons.name for cons in metadata_unique_constraints + if cons.name is not None]) + + unnamed_metadata_uqs = set([ + compare._uq_constraint_sig(cons).sig + for cons in metadata_unique_constraints + if cons.name is None + ]) + + metadata_ix_names = set([ + cons.name for cons in metadata_indexes if cons.unique]) + conn_uq_names = dict( + (cons.name, cons) for cons in conn_unique_constraints + ) + conn_ix_names = dict( + (cons.name, cons) for cons in conn_indexes if cons.unique + ) + + for overlap in set(conn_uq_names).intersection(conn_ix_names): + if overlap not in metadata_uq_names: + if compare._uq_constraint_sig(conn_uq_names[overlap]).sig \ + not in unnamed_metadata_uqs: + + conn_unique_constraints.discard(conn_uq_names[overlap]) + elif overlap not in metadata_ix_names: + conn_indexes.discard(conn_ix_names[overlap]) + class MySQLAlterDefault(AlterColumn): diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst index d8d2fa3..1d5fb93 100644 --- a/docs/build/changelog.rst +++ b/docs/build/changelog.rst @@ -6,6 +6,21 @@ Changelog .. changelog:: :version: 0.7.5 + .. change:: + :tags: bug, autogenerate, mysql + :tickets: 276 + + Fixed bug where MySQL backend would report dropped unique indexes + and/or constraints as both at the same time. This is because + MySQL doesn't actually have a "unique constraint" construct that + reports differently than a "unique index", so it is present in both + lists. The net effect though is that the MySQL backend will report + a dropped unique index/constraint as an index in cases where the object + was first created as a unique constraint, if no other information + is available to make the decision. This differs from other backends + like Postgresql which can report on unique constraints and + unique indexes separately. + .. change:: :tags: bug, commands :tickets: 269 diff --git a/tests/requirements.py b/tests/requirements.py index 37fd4e0..0304919 100644 --- a/tests/requirements.py +++ b/tests/requirements.py @@ -49,3 +49,7 @@ class DefaultRequirements(SuiteRequirements): def fk_names(self): """foreign key constraints always have names in the DB""" return exclusions.fails_on('sqlite') + + @property + def reflects_unique_constraints_unambiguously(self): + return exclusions.fails_on("mysql") diff --git a/tests/test_autogen_indexes.py b/tests/test_autogen_indexes.py index 702e2df..f798dd3 100644 --- a/tests/test_autogen_indexes.py +++ b/tests/test_autogen_indexes.py @@ -36,6 +36,8 @@ class NoUqReflection(object): class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase): reports_unique_constraints = True + reports_unique_constraints_as_indexes = False + __requires__ = ('unique_constraint_reflection', ) __only_on__ = 'sqlite' @@ -441,8 +443,17 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase): ('x' in obj.name) if obj.name is not None else False) for cmd, obj in diffs) if self.reports_unnamed_constraints: - assert ("remove_constraint", True) in diffs - assert ("add_constraint", False) in diffs + if self.reports_unique_constraints_as_indexes: + eq_( + diffs, + set([("remove_index", True), ("add_constraint", False)]) + ) + else: + eq_( + diffs, + set([("remove_constraint", True), + ("add_constraint", False)]) + ) def test_remove_named_unique_index(self): m1 = MetaData() @@ -453,14 +464,14 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase): Index('xidx', 'x', unique=True) ) Table('remove_idx', m2, - Column('x', Integer), + Column('x', Integer) ) diffs = self._fixture(m1, m2) if self.reports_unique_constraints: diffs = set((cmd, obj.name) for cmd, obj in diffs) - assert ("remove_index", "xidx") in diffs + eq_(diffs, set([("remove_index", "xidx")])) else: eq_(diffs, []) @@ -479,8 +490,11 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase): diffs = self._fixture(m1, m2) if self.reports_unique_constraints: - diffs = ((cmd, obj.name) for cmd, obj in diffs) - assert ("remove_constraint", "xidx") in diffs + diffs = set((cmd, obj.name) for cmd, obj in diffs) + if self.reports_unique_constraints_as_indexes: + eq_(diffs, set([("remove_index", "xidx")])) + else: + eq_(diffs, set([("remove_constraint", "xidx")])) else: eq_(diffs, []) @@ -628,6 +642,7 @@ class PGUniqueIndexTest(AutogenerateUniqueIndexTest): class MySQLUniqueIndexTest(AutogenerateUniqueIndexTest): reports_unnamed_constraints = True + reports_unique_constraints_as_indexes = True __only_on__ = 'mysql' def test_removed_idx_index_named_as_column(self): @@ -640,7 +655,6 @@ class MySQLUniqueIndexTest(AutogenerateUniqueIndexTest): assert False, "unexpected success" - class NoUqReflectionIndexTest(NoUqReflection, AutogenerateUniqueIndexTest): reports_unique_constraints = False __only_on__ = 'sqlite' @@ -765,6 +779,7 @@ class IncludeHooksTest(AutogenFixtureTest, TestBase): eq_(len(diffs), 1) @config.requirements.unique_constraint_reflection + @config.requirements.reflects_unique_constraints_unambiguously def test_remove_connection_uq(self): m1 = MetaData() m2 = MetaData() -- cgit v1.2.1