Detect and render autoincrement for alter_column()

The ``autoincrement=True`` flag is now rendered within the
:meth:`.Operations.alter_column` operation if the source column indicates
that this flag should be set to True.  The behavior is sensitive to
the SQLAlchemy version in place, as the "auto" default option is new
in SQLAlchemy 1.1.  When the source column indicates autoincrement
as True or "auto", the flag will render as True if the original column
contextually indicates that it should have "autoincrement" keywords,
and when the source column explcitly sets it to False, this is also
rendered.  The behavior is intended to preserve the AUTO_INCREMENT flag
on MySQL as the column is fully recreated on this backend.  Note that this
flag does **not** support alteration of a column's "autoincrement" status,
as this is not portable across backends.

Change-Id: I746c4841752adf9342bfdca7c9255aae5110b2ef
Fixes: #413

diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py
index 267b017ee7c27126f4d9d337136593e4b3161b2f..e23b466fd83c2a6627aef48452ec93df5987f8b4 100644 (file)
--- a/alembic/autogenerate/compare.py
+++ b/alembic/autogenerate/compare.py
@@ -669,6 +669,19 @@ def _compare_nullable(
                  )
 
 
+@comparators.dispatch_for("column")
+def _setup_autoincrement(
+    autogen_context, alter_column_op, schema, tname, cname, conn_col,
+        metadata_col):
+
+    if metadata_col.table._autoincrement_column is metadata_col:
+        alter_column_op.kw['autoincrement'] = True
+    elif util.sqla_110 and metadata_col.autoincrement is True:
+        alter_column_op.kw['autoincrement'] = True
+    elif metadata_col.autoincrement is False:
+        alter_column_op.kw['autoincrement'] = False
+
+
 @comparators.dispatch_for("column")
 def _compare_type(
     autogen_context, alter_column_op, schema, tname, cname, conn_col,

diff --git a/alembic/autogenerate/render.py b/alembic/autogenerate/render.py
index 6e10792a004786b98e0ac982eb88df93c1f44bd6..94a14676691ad6512fcb20cfeacfe1a161c74a47 100644 (file)
--- a/alembic/autogenerate/render.py
+++ b/alembic/autogenerate/render.py
@@ -331,6 +331,7 @@ def _alter_column(autogen_context, op):
     server_default = op.modify_server_default
     type_ = op.modify_type
     nullable = op.modify_nullable
+    autoincrement = op.kw.get('autoincrement', None)
     existing_type = op.existing_type
     existing_nullable = op.existing_nullable
     existing_server_default = op.existing_server_default
@@ -366,6 +367,9 @@ def _alter_column(autogen_context, op):
     if nullable is None and existing_nullable is not None:
         text += ",\n%sexisting_nullable=%r" % (
             indent, existing_nullable)
+    if autoincrement is not None:
+        text += ",\n%sautoincrement=%r" % (
+            indent, autoincrement)
     if server_default is False and existing_server_default:
         rendered = _render_server_default(
             existing_server_default,

diff --git a/alembic/testing/requirements.py b/alembic/testing/requirements.py
index ad4f27ccc953b4e7ba2012e34d49f8818845a0ba..be46a33189621e8f6f90996f4828826801e85e71 100644 (file)
--- a/alembic/testing/requirements.py
+++ b/alembic/testing/requirements.py
@@ -103,6 +103,13 @@ class SuiteRequirements(Requirements):
             "SQLAlchemy 0.9.9 or greater required"
         )
 
+    @property
+    def fail_before_sqla_110(self):
+        return exclusions.fails_if(
+            lambda config: not util.sqla_110,
+            "SQLAlchemy 1.1.0 or greater required"
+        )
+
     @property
     def sqlalchemy_08(self):
 

diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst
index caf3f909f1351ad01f439c97b7539a08b5388905..12f22420dc4e4883b0b872f8921e2e9211c32038 100644 (file)
--- a/docs/build/changelog.rst
+++ b/docs/build/changelog.rst
@@ -7,6 +7,23 @@ Changelog
     :version: 0.9.0
     :released:
 
+    .. change:: 413
+      :tags: bug, autogenerate, mysql
+      :tickets: 413
+
+      The ``autoincrement=True`` flag is now rendered within the
+      :meth:`.Operations.alter_column` operation if the source column indicates
+      that this flag should be set to True.  The behavior is sensitive to
+      the SQLAlchemy version in place, as the "auto" default option is new
+      in SQLAlchemy 1.1.  When the source column indicates autoincrement
+      as True or "auto", the flag will render as True if the original column
+      contextually indicates that it should have "autoincrement" keywords,
+      and when the source column explcitly sets it to False, this is also
+      rendered.  The behavior is intended to preserve the AUTO_INCREMENT flag
+      on MySQL as the column is fully recreated on this backend.  Note that this
+      flag does **not** support alteration of a column's "autoincrement" status,
+      as this is not portable across backends.
+
     .. change:: 85
       :tags: bug, postgresql
       :tickets: 85

diff --git a/tests/_autogen_fixtures.py b/tests/_autogen_fixtures.py
index 93d7b5eb0f45855ae77865a76d6454a93d6c4319..f731a7fa54c1cdf61b2b622bb04c1eba64fdc290 100644 (file)
--- a/tests/_autogen_fixtures.py
+++ b/tests/_autogen_fixtures.py
@@ -210,7 +210,8 @@ class AutogenFixtureTest(_ComparesFKs):
 
     def _fixture(
             self, m1, m2, include_schemas=False,
-            opts=None, object_filters=_default_object_filters):
+            opts=None, object_filters=_default_object_filters,
+            return_ops=False):
         self.metadata, model_metadata = m1, m2
         self.metadata.create_all(self.bind)
 
@@ -238,7 +239,11 @@ class AutogenFixtureTest(_ComparesFKs):
             autogenerate._produce_net_changes(
                 autogen_context, uo
             )
-            return uo.as_diffs()
+
+            if return_ops:
+                return uo
+            else:
+                return uo.as_diffs()
 
     reports_unnamed_constraints = False
 

diff --git a/tests/requirements.py b/tests/requirements.py
index 987c0c764b69471c0617db3f6b34e3fd58f1e03e..522ac5bf8a0ed693fce95452ebd15ae9b76bf658 100644 (file)
--- a/tests/requirements.py
+++ b/tests/requirements.py
@@ -114,3 +114,12 @@ class DefaultRequirements(SuiteRequirements):
                 return False
 
         return exclusions.only_if(check_uuid_ossp)
+
+    @property
+    def autoincrement_on_composite_pk(self):
+        return exclusions.skip_if(["sqlite"], "not supported by database")
+
+    @property
+    def integer_subtype_comparisons(self):
+        """if a compare of Integer and BigInteger is supported yet."""
+        return exclusions.skip_if(["oracle"], "not supported by alembic impl")

diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py
index 4816134fbc5a46c00bec2ecf10bc167e1af340b8..60dc6e25d7836ed58e1a5f8c0560eec4ea99ddab 100644 (file)
--- a/tests/test_autogen_diffs.py
+++ b/tests/test_autogen_diffs.py
@@ -754,9 +754,10 @@ class PKConstraintUpgradesIgnoresNullableTest(AutogenTest, TestBase):
         return cls._get_db_schema()
 
     def test_no_change(self):
-        diffs = []
+        uo = ops.UpgradeOps(ops=[])
         ctx = self.autogen_context
-        autogenerate._produce_net_changes(ctx, diffs)
+        autogenerate._produce_net_changes(ctx, uo)
+        diffs = uo.as_diffs()
         eq_(diffs, [])
 
 
@@ -1203,3 +1204,174 @@ class OrigObjectTest(TestBase):
         is_(op.to_index(), self.ix)
         is_(op.reverse().to_index(), self.ix)
 
+
+class AutoincrementTest(AutogenFixtureTest, TestBase):
+    __backend__ = True
+    __requires__ = 'integer_subtype_comparisons',
+
+    def test_alter_column_autoincrement_none(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table('a', m1, Column('x', Integer, nullable=False))
+        Table('a', m2, Column('x', Integer, nullable=True))
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        assert 'autoincrement' not in ops.ops[0].ops[0].kw
+
+    def test_alter_column_autoincrement_pk_false(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('x', Integer, primary_key=True, autoincrement=False))
+        Table(
+            'a', m2,
+            Column('x', BigInteger, primary_key=True, autoincrement=False))
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        is_(ops.ops[0].ops[0].kw['autoincrement'], False)
+
+    def test_alter_column_autoincrement_pk_implicit_true(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('x', Integer, primary_key=True))
+        Table(
+            'a', m2,
+            Column('x', BigInteger, primary_key=True))
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        is_(ops.ops[0].ops[0].kw['autoincrement'], True)
+
+    def test_alter_column_autoincrement_pk_explicit_true(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('x', Integer, primary_key=True, autoincrement=True))
+        Table(
+            'a', m2,
+            Column('x', BigInteger, primary_key=True, autoincrement=True))
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        is_(ops.ops[0].ops[0].kw['autoincrement'], True)
+
+    def test_alter_column_autoincrement_nonpk_false(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('id', Integer, primary_key=True),
+            Column('x', Integer, autoincrement=False)
+        )
+        Table(
+            'a', m2,
+            Column('id', Integer, primary_key=True),
+            Column('x', BigInteger, autoincrement=False)
+        )
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        is_(ops.ops[0].ops[0].kw['autoincrement'], False)
+
+    def test_alter_column_autoincrement_nonpk_implicit_false(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('id', Integer, primary_key=True),
+            Column('x', Integer)
+        )
+        Table(
+            'a', m2,
+            Column('id', Integer, primary_key=True),
+            Column('x', BigInteger)
+        )
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        assert 'autoincrement' not in ops.ops[0].ops[0].kw
+
+    @config.requirements.fail_before_sqla_110
+    def test_alter_column_autoincrement_nonpk_explicit_true(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('id', Integer, primary_key=True),
+            Column('x', Integer, autoincrement=True)
+        )
+        Table(
+            'a', m2,
+            Column('id', Integer, primary_key=True),
+            Column('x', BigInteger, autoincrement=True)
+        )
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        is_(ops.ops[0].ops[0].kw['autoincrement'], True)
+
+    def test_alter_column_autoincrement_compositepk_false(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('id', Integer, primary_key=True),
+            Column('x', Integer, primary_key=True, autoincrement=False)
+        )
+        Table(
+            'a', m2,
+            Column('id', Integer, primary_key=True),
+            Column('x', BigInteger, primary_key=True, autoincrement=False)
+        )
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        is_(ops.ops[0].ops[0].kw['autoincrement'], False)
+
+    def test_alter_column_autoincrement_compositepk_implicit_false(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('id', Integer, primary_key=True),
+            Column('x', Integer, primary_key=True)
+        )
+        Table(
+            'a', m2,
+            Column('id', Integer, primary_key=True),
+            Column('x', BigInteger, primary_key=True)
+        )
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        assert 'autoincrement' not in ops.ops[0].ops[0].kw
+
+    @config.requirements.autoincrement_on_composite_pk
+    def test_alter_column_autoincrement_compositepk_explicit_true(self):
+        m1 = MetaData()
+        m2 = MetaData()
+
+        Table(
+            'a', m1,
+            Column('id', Integer, primary_key=True, autoincrement=False),
+            Column('x', Integer, primary_key=True, autoincrement=True),
+            # on SQLA 1.0 and earlier, this being present
+            # trips the "add KEY for the primary key" so that the
+            # AUTO_INCREMENT keyword is accepted by MySQL.  SQLA 1.1 and
+            # greater the columns are just reorganized.
+            mysql_engine='InnoDB'
+        )
+        Table(
+            'a', m2,
+            Column('id', Integer, primary_key=True, autoincrement=False),
+            Column('x', BigInteger, primary_key=True, autoincrement=True)
+        )
+
+        ops = self._fixture(m1, m2, return_ops=True)
+        is_(ops.ops[0].ops[0].kw['autoincrement'], True)

diff --git a/tests/test_autogen_render.py b/tests/test_autogen_render.py
index 7689d365138136340badfa37b5a18babf6e76dd3..03d8d040e98e601ef1719b0ca1ef50b25ef7aaae 100644 (file)
--- a/tests/test_autogen_render.py
+++ b/tests/test_autogen_render.py
@@ -4,7 +4,7 @@ from alembic.testing import TestBase, exclusions, assert_raises
 
 from alembic.operations import ops
 from sqlalchemy import MetaData, Column, Table, String, \
-    Numeric, CHAR, ForeignKey, DATETIME, Integer, \
+    Numeric, CHAR, ForeignKey, DATETIME, Integer, BigInteger, \
     CheckConstraint, Unicode, Enum, cast,\
     UniqueConstraint, Boolean, ForeignKeyConstraint,\
     PrimaryKeyConstraint, Index, func, text, DefaultClause
@@ -1074,6 +1074,19 @@ class AutogenRenderTest(TestBase):
             "existing_type=sa.Integer(), nullable=True, schema='foo')"
         )
 
+    def test_render_modify_type_w_autoincrement(self):
+        op_obj = ops.AlterColumnOp(
+            "sometable", "somecolumn",
+            modify_type=Integer(), existing_type=BigInteger(),
+            autoincrement=True
+        )
+        eq_ignore_whitespace(
+            autogenerate.render_op_text(self.autogen_context, op_obj),
+            "op.alter_column('sometable', 'somecolumn', "
+            "existing_type=sa.BigInteger(), type_=sa.Integer(), "
+            "autoincrement=True)"
+        )
+
     def test_render_fk_constraint_kwarg(self):
         m = MetaData()
         t1 = Table('t', m, Column('c', Integer))