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: sqlalchemy#788

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

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.

tests/test_autogen_diffs.py

@@ -33,6 +33,7 @@
 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 @@ def test_column_type_modified_custom_compare_type_returns_True(self):
 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 @@ def test_no_change(self):
         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"