diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py index e34114b1..7ae7b619 100644 --- 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 index 00000000..6844cea5 --- /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 d847e1e0..a2215aff 100644 --- a/tests/test_autogen_diffs.py +++ b/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"