Remove obsolete SQLAlchemy pk issue workaround

Fixed issue where autogenerate rendering of ``op.alter_column()`` would
fail to include MySQL ``existing_nullable=False`` if the column were part
of a primary key constraint within the table metadata.

Change-Id: I6f3473b66c0faee035a438a25dcbe3a6f24eb041
Fixes: #788

diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py
index e34114b199da3e8ba8e6e91441db5f80af60fffb..7ae7b6191f762a9b18461bad14607426851b305f 100644 (file)
--- a/alembic/autogenerate/compare.py
+++ b/alembic/autogenerate/compare.py
@@ -838,10 +838,6 @@ def _compare_nullable(
     metadata_col,
 ):
 
-    # work around SQLAlchemy issue #3023
-    if metadata_col.primary_key:
-        return
-
     metadata_col_nullable = metadata_col.nullable
     conn_col_nullable = conn_col.nullable
     alter_column_op.existing_nullable = conn_col_nullable

diff --git a/docs/build/unreleased/788.rst b/docs/build/unreleased/788.rst
new file mode 100644 (file)
index 0000000..6844cea
--- /dev/null
+++ b/docs/build/unreleased/788.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, mysql, autogenerate
+    :tickets: 788
+
+    Fixed issue where autogenerate rendering of ``op.alter_column()`` would
+    fail to include MySQL ``existing_nullable=False`` if the column were part
+    of a primary key constraint within the table metadata.

diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py
index d847e1e0dc6d8b783c63d5cf225870c2b04b9ee8..a2215affa82cc2527b578df8e08bc1c58499f2bb 100644 (file)
--- a/tests/test_autogen_diffs.py
+++ b/tests/test_autogen_diffs.py
@@ -33,6 +33,7 @@ from sqlalchemy import UniqueConstraint
 from sqlalchemy import VARCHAR
 from sqlalchemy.dialects import mysql
 from sqlalchemy.dialects import sqlite
+from sqlalchemy.testing import in_
 from sqlalchemy.types import NULLTYPE
 from sqlalchemy.types import VARBINARY
 
@@ -1190,7 +1191,8 @@ class AutogenerateCustomCompareTypeTest(AutogenTest, TestBase):
 class PKConstraintUpgradesIgnoresNullableTest(AutogenTest, TestBase):
     __backend__ = True
 
-    # test workaround for SQLAlchemy issue #3023, alembic issue #199
+    # test behavior for issue originally observed in SQLAlchemy issue #3023,
+    # alembic issue #199
     @classmethod
     def _get_db_schema(cls):
         m = MetaData()
@@ -1216,6 +1218,81 @@ class PKConstraintUpgradesIgnoresNullableTest(AutogenTest, TestBase):
         eq_(diffs, [])
 
 
+class AlterColumnTest(AutogenFixtureTest, TestBase):
+    __backend__ = True
+
+    @testing.combinations((True,), (False,))
+    @config.requirements.comments
+    def test_all_existings_filled(self, pk):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table("a", m1, Column("x", Integer, primary_key=pk))
+        Table("a", m2, Column("x", Integer, comment="x", primary_key=pk))
+
+        alter_col = self._assert_alter_col(m1, m2, pk)
+        eq_(alter_col.modify_comment, "x")
+
+    @testing.combinations((True,), (False,))
+    @config.requirements.comments
+    def test_all_existings_filled_in_notnull(self, pk):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table("a", m1, Column("x", Integer, nullable=False, primary_key=pk))
+        Table(
+            "a",
+            m2,
+            Column("x", Integer, nullable=False, comment="x", primary_key=pk),
+        )
+
+        self._assert_alter_col(m1, m2, pk, nullable=False)
+
+    @testing.combinations((True,), (False,))
+    @config.requirements.comments
+    def test_all_existings_filled_in_comment(self, pk):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table("a", m1, Column("x", Integer, comment="old", primary_key=pk))
+        Table("a", m2, Column("x", Integer, comment="new", primary_key=pk))
+
+        alter_col = self._assert_alter_col(m1, m2, pk)
+        eq_(alter_col.existing_comment, "old")
+
+    @testing.combinations((True,), (False,))
+    @config.requirements.comments
+    def test_all_existings_filled_in_server_default(self, pk):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            "a", m1, Column("x", Integer, server_default="5", primary_key=pk)
+        )
+        Table(
+            "a",
+            m2,
+            Column(
+                "x", Integer, server_default="5", comment="new", primary_key=pk
+            ),
+        )
+
+        alter_col = self._assert_alter_col(m1, m2, pk)
+        in_("5", alter_col.existing_server_default.arg.text)
+
+    def _assert_alter_col(self, m1, m2, pk, nullable=None):
+        ops = self._fixture(m1, m2, return_ops=True)
+        modify_table = ops.ops[-1]
+        alter_col = modify_table.ops[0]
+
+        if nullable is None:
+            eq_(alter_col.existing_nullable, not pk)
+        else:
+            eq_(alter_col.existing_nullable, nullable)
+        assert alter_col.existing_type._compare_type_affinity(Integer())
+        return alter_col
+
+
 class AutogenKeyTest(AutogenTest, TestBase):
     __only_on__ = "sqlite"