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

diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py
index d932736070820052b4447236577d609ad461e3a8..cfa110ac93a31d8c4be58d48f0ade1358c3cc144 100644 (file)
--- a/alembic/autogenerate/compare.py
+++ b/alembic/autogenerate/compare.py
@@ -219,12 +219,16 @@ def _compare_indexes_and_uniques(schema, tname, object_filters, conn_table,
     metadata_indexes = set(metadata_table.indexes)
 
     conn_uniques = conn_indexes = frozenset()
+
+    supports_unique_constraints = False
+
     if conn_table is not None:
         # 1b. ... and from connection, if the table exists
         if hasattr(inspector, "get_unique_constraints"):
             try:
                 conn_uniques = inspector.get_unique_constraints(
                                                 tname, schema=schema)
+                supports_unique_constraints = True
             except NotImplementedError:
                 pass
         try:
@@ -307,6 +311,10 @@ def _compare_indexes_and_uniques(schema, tname, object_filters, conn_table,
                     ])
             )
         else:
+            if not supports_unique_constraints:
+                # can't report unique indexes as added if we don't
+                # detect them
+                return
             if is_create_table:
                 # unique constraints are created inline with table defs
                 return
@@ -319,6 +327,12 @@ def _compare_indexes_and_uniques(schema, tname, object_filters, conn_table,
 
     def obj_removed(obj):
         if obj.is_index:
+            if obj.is_unique and not supports_unique_constraints:
+                # many databases double up unique constraints
+                # as unique indexes.  without that list we can't
+                # be sure what we're doing here
+                return
+
             diffs.append(("remove_index", obj.const))
             log.info("Detected removed index '%s' on '%s'", obj.name, tname)
         else:
@@ -341,7 +355,6 @@ def _compare_indexes_and_uniques(schema, tname, object_filters, conn_table,
             diffs.append(("remove_constraint", old.const))
             diffs.append(("add_constraint", new.const))
 
-
     for added_name in sorted(set(metadata_names).difference(conn_names)):
         obj = metadata_names[added_name]
         obj_added(obj)

diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst
index b090a1f660738e8f7200afe35fb89b6d6b073f78..caaa5c9a30785e4f54eca385666859e1a33c8d42 100644 (file)
--- a/docs/build/changelog.rst
+++ b/docs/build/changelog.rst
@@ -5,6 +5,22 @@ Changelog
 .. changelog::
     :version: 0.6.4
 
+    .. change::
+      :tags: bug, mssql
+      :tickets: 185
+
+      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.
+
     .. change::
       :tags: bug
       :tickets: 183

diff --git a/tests/test_autogenerate.py b/tests/test_autogenerate.py
index 2d41f94f96bc5fb744fdb0e3920d2f3966e10e34..34875db68029416d6a226db82930904bdf9985da 100644 (file)
--- a/tests/test_autogenerate.py
+++ b/tests/test_autogenerate.py
@@ -764,6 +764,7 @@ class AutogenerateDiffTestWSchema(ModelOne, AutogenTest, TestCase):
 
 
 class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestCase):
+    reports_unique_constraints = True
 
     def test_index_flag_becomes_named_unique_constraint(self):
         m1 = MetaData()
@@ -784,11 +785,15 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestCase):
 
         diffs = self._fixture(m1, m2)
 
-        eq_(diffs[0][0], "add_constraint")
-        eq_(diffs[0][1].name, "uq_user_name")
+        if self.reports_unique_constraints:
+            eq_(diffs[0][0], "add_constraint")
+            eq_(diffs[0][1].name, "uq_user_name")
 
-        eq_(diffs[1][0], "remove_index")
-        eq_(diffs[1][1].name, "ix_user_name")
+            eq_(diffs[1][0], "remove_index")
+            eq_(diffs[1][1].name, "ix_user_name")
+        else:
+            eq_(diffs[0][0], "remove_index")
+            eq_(diffs[0][1].name, "ix_user_name")
 
 
     def test_add_unique_constraint(self):
@@ -807,8 +812,12 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestCase):
         )
 
         diffs = self._fixture(m1, m2)
-        eq_(diffs[0][0], "add_constraint")
-        eq_(diffs[0][1].name, "uq_email_address")
+
+        if self.reports_unique_constraints:
+            eq_(diffs[0][0], "add_constraint")
+            eq_(diffs[0][1].name, "uq_email_address")
+        else:
+            eq_(diffs, [])
 
 
     def test_index_becomes_unique(self):
@@ -844,6 +853,7 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestCase):
         eq_(diffs[1][1].unique, True)
 
 
+
     def test_mismatch_db_named_col_flag(self):
         m1 = MetaData()
         m2 = MetaData()
@@ -898,12 +908,14 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestCase):
 
         diffs = self._fixture(m1, m2)
 
-        eq_(diffs[0][0], "remove_constraint")
-        eq_(diffs[0][1].name, "nochange")
-
-        eq_(diffs[1][0], "add_constraint")
-        eq_(diffs[1][1].name, "nochange")
+        if self.reports_unique_constraints:
+            eq_(diffs[0][0], "remove_constraint")
+            eq_(diffs[0][1].name, "nochange")
 
+            eq_(diffs[1][0], "add_constraint")
+            eq_(diffs[1][1].name, "nochange")
+        else:
+            eq_(diffs, [])
 
     def test_nothing_changed_one(self):
         m1 = MetaData()
@@ -993,8 +1005,11 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestCase):
 
         diffs = self._fixture(m1, m2)
 
-        diffs = set((cmd, obj.name) for cmd, obj in diffs)
-        assert ("remove_index", "xidx") in diffs
+        if self.reports_unique_constraints:
+            diffs = set((cmd, obj.name) for cmd, obj in diffs)
+            assert ("remove_index", "xidx") in diffs
+        else:
+            eq_(diffs, [])
 
 
     def test_remove_named_unique_constraint(self):
@@ -1011,8 +1026,11 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestCase):
 
         diffs = self._fixture(m1, m2)
 
-        diffs = ((cmd, obj.name) for cmd, obj in diffs)
-        assert ("remove_constraint", "xidx") in diffs
+        if self.reports_unique_constraints:
+            diffs = ((cmd, obj.name) for cmd, obj in diffs)
+            assert ("remove_constraint", "xidx") in diffs
+        else:
+            eq_(diffs, [])
 
     def test_dont_add_uq_on_table_create(self):
         m1 = MetaData()
@@ -1145,6 +1163,108 @@ class MySQLUniqueIndexTest(AutogenerateUniqueIndexTest):
     def _get_bind(cls):
         return db_for_dialect('mysql')
 
+class NoUqReflectionIndexTest(AutogenerateUniqueIndexTest):
+    reports_unique_constraints = False
+
+    @classmethod
+    def _get_bind(cls):
+        eng = sqlite_db()
+
+        def unimpl(*arg, **kw):
+            raise NotImplementedError()
+        eng.dialect.get_unique_constraints = unimpl
+        return eng
+
+    def test_unique_not_reported(self):
+        m1 = MetaData()
+        Table('order', m1,
+            Column('order_id', Integer, primary_key=True),
+            Column('amount', Numeric(10, 2), nullable=True),
+            Column('user_id', Integer),
+            UniqueConstraint('order_id', 'user_id',
+                name='order_order_id_user_id_unique'
+            )
+        )
+
+        diffs = self._fixture(m1, m1)
+        eq_(diffs, [])
+
+    def test_remove_unique_index_not_reported(self):
+        m1 = MetaData()
+        Table('order', m1,
+            Column('order_id', Integer, primary_key=True),
+            Column('amount', Numeric(10, 2), nullable=True),
+            Column('user_id', Integer),
+            Index('oid_ix', 'order_id', 'user_id',
+                unique=True
+            )
+        )
+        m2 = MetaData()
+        Table('order', m2,
+            Column('order_id', Integer, primary_key=True),
+            Column('amount', Numeric(10, 2), nullable=True),
+            Column('user_id', Integer),
+        )
+
+        diffs = self._fixture(m1, m2)
+        eq_(diffs, [])
+
+    def test_remove_plain_index_is_reported(self):
+        m1 = MetaData()
+        Table('order', m1,
+            Column('order_id', Integer, primary_key=True),
+            Column('amount', Numeric(10, 2), nullable=True),
+            Column('user_id', Integer),
+            Index('oid_ix', 'order_id', 'user_id')
+        )
+        m2 = MetaData()
+        Table('order', m2,
+            Column('order_id', Integer, primary_key=True),
+            Column('amount', Numeric(10, 2), nullable=True),
+            Column('user_id', Integer),
+        )
+
+        diffs = self._fixture(m1, m2)
+        eq_(diffs[0][0], 'remove_index')
+
+
+class NoUqReportsIndAsUqTest(NoUqReflectionIndexTest):
+    """this test suite simulates the condition where:
+
+    a. the dialect doesn't report unique constraints
+
+    b. the dialect returns unique constraints within the indexes list.
+
+    Currently the mssql dialect does this, but here we force this
+    condition so that we can test the behavior regardless of if/when
+    mssql supports unique constraint reflection.
+
+    """
+
+    @classmethod
+    def _get_bind(cls):
+        eng = sqlite_db()
+
+        _get_unique_constraints = eng.dialect.get_unique_constraints
+        _get_indexes = eng.dialect.get_indexes
+
+        def unimpl(*arg, **kw):
+            raise NotImplementedError()
+
+        def get_indexes(self, connection, tablename, **kw):
+            indexes = _get_indexes(self, connection, tablename, **kw)
+            for uq in _get_unique_constraints(
+                            self, connection, tablename, **kw
+                            ):
+                uq['unique'] = True
+                indexes.append(uq)
+            return indexes
+
+        eng.dialect.get_unique_constraints = unimpl
+        eng.dialect.get_indexes = get_indexes
+        return eng
+
+
 
 class AutogenerateCustomCompareTypeTest(AutogenTest, TestCase):
     @classmethod