Detect indexes for table that's dropped

Fixed bug where the indexes would not be included in a
migration that was dropping the owning table.   The fix
now will also emit DROP INDEX for the indexes ahead of time,
but more importantly will include CREATE INDEX in the
downgrade migration.

Change-Id: I15852bf1cca6de26dd6e7e5ede69bc9e2145c5c0
Fixes: #468

diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py
index 3abb32dfeb39322fb39be2ebef39a5cdcc63819f..1b660613fd17d87e51c3b6489c8de7bed30346c7 100644 (file)
--- a/alembic/autogenerate/compare.py
+++ b/alembic/autogenerate/compare.py
@@ -136,6 +136,16 @@ def _compare_tables(conn_table_names, metadata_table_names,
                 _compat_autogen_column_reflect(inspector))
             inspector.reflecttable(t, None)
         if autogen_context.run_filters(t, tname, "table", True, None):
+
+            modify_table_ops = ops.ModifyTableOps(tname, [], schema=s)
+
+            comparators.dispatch("table")(
+                autogen_context, modify_table_ops,
+                s, tname, t, None
+            )
+            if not modify_table_ops.is_empty():
+                upgrade_ops.ops.append(modify_table_ops)
+
             upgrade_ops.ops.append(
                 ops.DropTableOp.from_table(t)
             )
@@ -362,13 +372,18 @@ def _compare_indexes_and_uniques(
 
     inspector = autogen_context.inspector
     is_create_table = conn_table is None
+    is_drop_table = metadata_table is None
 
     # 1a. get raw indexes and unique constraints from metadata ...
-    metadata_unique_constraints = set(
-        uq for uq in metadata_table.constraints
-        if isinstance(uq, sa_schema.UniqueConstraint)
-    )
-    metadata_indexes = set(metadata_table.indexes)
+    if metadata_table is not None:
+        metadata_unique_constraints = set(
+            uq for uq in metadata_table.constraints
+            if isinstance(uq, sa_schema.UniqueConstraint)
+        )
+        metadata_indexes = set(metadata_table.indexes)
+    else:
+        metadata_unique_constraints = set()
+        metadata_indexes = set()
 
     conn_uniques = conn_indexes = frozenset()
 
@@ -401,8 +416,13 @@ def _compare_indexes_and_uniques(
 
         # 2. convert conn-level objects from raw inspector records
         # into schema objects
-        conn_uniques = set(_make_unique_constraint(uq_def, conn_table)
-                           for uq_def in conn_uniques)
+        if is_drop_table:
+            # for DROP TABLE uniques are inline, don't need them
+            conn_uniques = set()
+        else:
+            conn_uniques = set(_make_unique_constraint(uq_def, conn_table)
+                               for uq_def in conn_uniques)
+
         conn_indexes = set(_make_index(ix, conn_table) for ix in conn_indexes)
 
     # 2a. if the dialect dupes unique indexes as unique constraints
@@ -493,7 +513,7 @@ def _compare_indexes_and_uniques(
                 # can't report unique indexes as added if we don't
                 # detect them
                 return
-            if is_create_table:
+            if is_create_table or is_drop_table:
                 # unique constraints are created inline with table defs
                 return
             if autogen_context.run_filters(
@@ -523,6 +543,10 @@ def _compare_indexes_and_uniques(
                 log.info(
                     "Detected removed index '%s' on '%s'", obj.name, tname)
         else:
+            if is_create_table or is_drop_table:
+                # if the whole table is being dropped, we don't need to
+                # consider unique constraint separately
+                return
             if autogen_context.run_filters(
                     obj.const, obj.name,
                     "unique_constraint", True, None):
@@ -617,7 +641,6 @@ def _correct_for_uq_duplicates_uix(
         conn_indexes,
         metadata_unique_constraints,
         metadata_indexes):
-
     # dedupe unique indexes vs. constraints, since MySQL / Oracle
     # doesn't really have unique constraints as a separate construct.
     # but look in the metadata and try to maintain constructs
@@ -644,6 +667,7 @@ def _correct_for_uq_duplicates_uix(
         (cons.name, cons) for cons in conn_unique_constraints
         if cons.info['duplicates_index']
     )
+
     for overlap in uqs_dupe_indexes:
         if overlap not in metadata_uq_names:
             if _uq_constraint_sig(uqs_dupe_indexes[overlap]).sig \
@@ -775,7 +799,7 @@ def _compare_foreign_keys(
 
     # if we're doing CREATE TABLE, all FKs are created
     # inline within the table def
-    if conn_table is None:
+    if conn_table is None or metadata_table is None:
         return
 
     inspector = autogen_context.inspector

diff --git a/docs/build/unreleased/468.rst b/docs/build/unreleased/468.rst
new file mode 100644 (file)
index 0000000..3abc974
--- /dev/null
+++ b/docs/build/unreleased/468.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, autogenerate
+    :tickets: 468
+
+    Fixed bug where the indexes would not be included in a
+    migration that was dropping the owning table.   The fix
+    now will also emit DROP INDEX for the indexes ahead of time,
+    but more importantly will include CREATE INDEX in the
+    downgrade migration.

diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py
index db77d7451d9bedc1726315026e2a2a697db8427f..85f87a31699949bac0ec1385cf6b6c2d71c9c289 100644 (file)
--- a/tests/test_autogen_diffs.py
+++ b/tests/test_autogen_diffs.py
@@ -514,7 +514,10 @@ class AutogenerateDiffTest(ModelOne, AutogenTest, TestBase):
         )
         eq_(
             [(rec[0], rec[1].name) for rec in uo.as_diffs()],
-            [('remove_table', 'extra'), ('remove_table', 'user')]
+            [
+                ('remove_table', 'extra'),
+                ('remove_index', 'pw_idx'),
+                ('remove_table', 'user'), ]
         )
 
 

diff --git a/tests/test_autogen_indexes.py b/tests/test_autogen_indexes.py
index 2eaf7d6853b25c131484cd9b8666dc2810236861..00793007a092e6a916d5fd0e8c646de85232002d 100644 (file)
--- a/tests/test_autogen_indexes.py
+++ b/tests/test_autogen_indexes.py
@@ -424,6 +424,65 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase):
         diffs = self._fixture(m1, m2)
         eq_(diffs[0][0], 'remove_index')
 
+    def test_drop_table_w_indexes(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        t = Table(
+            'some_table', m1,
+            Column('id', Integer, primary_key=True),
+            Column('x', String(20)),
+            Column('y', String(20)),
+        )
+        Index('xy_idx', t.c.x, t.c.y)
+        Index('y_idx', t.c.y)
+
+        diffs = self._fixture(m1, m2)
+        eq_(diffs[0][0], 'remove_index')
+        eq_(diffs[1][0], 'remove_index')
+        eq_(diffs[2][0], 'remove_table')
+
+        eq_(
+            set([diffs[0][1].name, diffs[1][1].name]),
+            set(['xy_idx', 'y_idx'])
+        )
+
+    # this simply doesn't fully work before we had
+    # effective deduping of indexes/uniques.
+    @config.requirements.sqlalchemy_100
+    def test_drop_table_w_uq_constraint(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'some_table', m1,
+            Column('id', Integer, primary_key=True),
+            Column('x', String(20)),
+            Column('y', String(20)),
+            UniqueConstraint('y', name='uq_y')
+        )
+
+        diffs = self._fixture(m1, m2)
+
+        if self.reports_unique_constraints_as_indexes:
+            # for MySQL this UQ will look like an index, so
+            # make sure it at least sets it up correctly
+            eq_(diffs[0][0], 'remove_index')
+            eq_(diffs[1][0], 'remove_table')
+            eq_(len(diffs), 2)
+
+            constraints = [c for c in diffs[1][1].constraints
+                           if isinstance(c, UniqueConstraint)]
+            eq_(len(constraints), 0)
+        else:
+            eq_(diffs[0][0], 'remove_table')
+            eq_(len(diffs), 1)
+
+            constraints = [c for c in diffs[0][1].constraints
+                           if isinstance(c, UniqueConstraint)]
+            if self.reports_unique_constraints:
+                eq_(len(constraints), 1)
+
     def test_unnamed_cols_changed(self):
         m1 = MetaData()
         m2 = MetaData()