From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 20 May 2021 17:06:52 +0000 (-0400)
Subject: Remove unique/index flags when copying table
X-Git-Tag: rel_1_6_3~2^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=acc82bff397be712a7e4f5e2069e7bfbf93ff5a9;p=thirdparty%2Fsqlalchemy%2Falembic.git

Remove unique/index flags when copying table

Fixed 1.6-series regression where ``UniqueConstraint`` and to a lesser
extent ``Index`` objects would be doubled up in the generated model when
the ``unique=True`` / ``index=True`` flags were used.

Change-Id: Idb21891d66cc8d46b257f53062660ce1d4825fb8
Fixes: #844
---

diff --git a/alembic/operations/ops.py b/alembic/operations/ops.py
index 9283a2d6..edd13b80 100644
--- a/alembic/operations/ops.py
+++ b/alembic/operations/ops.py
@@ -987,6 +987,7 @@ class CreateTableOp(MigrateOperation):
             schema=self.schema,
             prefixes=self.prefixes,
             comment=self.comment,
+            _constraints_included=True,
             **self.kw
         )
 
@@ -1110,6 +1111,7 @@ class DropTableOp(MigrateOperation):
             self.table_name,
             *cols_and_constraints,
             schema=self.schema,
+            _constraints_included=True,
             **self.table_kw
         )
         return t
diff --git a/alembic/operations/schemaobj.py b/alembic/operations/schemaobj.py
index 5d04ee20..9572e191 100644
--- a/alembic/operations/schemaobj.py
+++ b/alembic/operations/schemaobj.py
@@ -149,6 +149,17 @@ class SchemaObjects(object):
             for c in columns
             if isinstance(c, Column)
         ]
+        # these flags have already added their UniqueConstraint /
+        # Index objects to the table, so flip them off here.
+        # SQLAlchemy tometadata() avoids this instead by preserving the
+        # flags and skipping the constraints that have _type_bound on them,
+        # but for a migration we'd rather list out the constraints
+        # explicitly.
+        _constraints_included = kw.pop("_constraints_included", False)
+        if _constraints_included:
+            for c in cols:
+                c.unique = c.index = False
+
         t = sa_schema.Table(name, m, *cols, **kw)
 
         constraints = [
diff --git a/docs/build/unreleased/844.rst b/docs/build/unreleased/844.rst
new file mode 100644
index 00000000..2818e095
--- /dev/null
+++ b/docs/build/unreleased/844.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, regression, autogenerate
+    :tickets: 844
+
+    Fixed 1.6-series regression where ``UniqueConstraint`` and to a lesser
+    extent ``Index`` objects would be doubled up in the generated model when
+    the ``unique=True`` / ``index=True`` flags were used.
diff --git a/tests/test_autogen_indexes.py b/tests/test_autogen_indexes.py
index 6fda2833..5ad65bae 100644
--- a/tests/test_autogen_indexes.py
+++ b/tests/test_autogen_indexes.py
@@ -8,6 +8,7 @@ from sqlalchemy import Index
 from sqlalchemy import Integer
 from sqlalchemy import MetaData
 from sqlalchemy import Numeric
+from sqlalchemy import PrimaryKeyConstraint
 from sqlalchemy import String
 from sqlalchemy import Table
 from sqlalchemy import UniqueConstraint
@@ -975,8 +976,34 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase):
 
         eq_(diffs[0][0], "add_table")
         eq_(len(diffs), 1)
-        assert UniqueConstraint in set(
-            type(c) for c in diffs[0][1].constraints
+
+        # checking for dupes also
+        eq_(
+            sorted(
+                [type(cons) for cons in diffs[0][1].constraints],
+                key=lambda c: c.__name__,
+            ),
+            [PrimaryKeyConstraint, UniqueConstraint],
+        )
+
+    @config.requirements.reflects_unique_constraints_unambiguously
+    def test_dont_add_uq_on_reverse_table_drop(self):
+        m1 = MetaData()
+        m2 = MetaData()
+        Table("no_uq", m1, Column("x", String(50), unique=True))
+        diffs = self._fixture(m1, m2)
+
+        eq_(diffs[0][0], "remove_table")
+        eq_(len(diffs), 1)
+
+        # because the drop comes from reflection, the "unique=True" flag
+        # is lost in any case.
+        eq_(
+            sorted(
+                [type(cons) for cons in diffs[0][1].constraints],
+                key=lambda c: c.__name__,
+            ),
+            [PrimaryKeyConstraint, UniqueConstraint],
         )
 
     def test_add_uq_ix_on_table_create(self):
@@ -990,8 +1017,15 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase):
         assert UniqueConstraint not in set(
             type(c) for c in diffs[0][1].constraints
         )
+
         eq_(diffs[1][0], "add_index")
-        eq_(diffs[1][1].unique, True)
+        d_table = diffs[0][1]
+        d_idx = diffs[1][1]
+        eq_(d_idx.unique, True)
+
+        # check for dupes
+        eq_(len(diffs), 2)
+        assert not d_table.indexes
 
     def test_add_ix_on_table_create(self):
         m1 = MetaData()
@@ -1293,6 +1327,11 @@ class NoUqReflectionIndexTest(NoUqReflection, AutogenerateUniqueIndexTest):
             "unique constraint reflection disabled for this suite"
         )
 
+    def test_dont_add_uq_on_reverse_table_drop(self):
+        config.skip_test(
+            "unique constraint reflection disabled for this suite"
+        )
+
     def test_unique_not_reported(self):
         m1 = MetaData()
         Table(