From: Mike Bayer Date: Wed, 22 Feb 2017 17:51:53 +0000 (-0500) Subject: Detect and render autoincrement for alter_column() X-Git-Tag: rel_0_9_0~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f99eb38ceb200b5d03776ab15d1d0473d7db83ea;p=thirdparty%2Fsqlalchemy%2Falembic.git 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 267b017e..e23b466f 100644 --- 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 6e10792a..94a14676 100644 --- 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 ad4f27cc..be46a331 100644 --- 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 caf3f909..12f22420 100644 --- 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 93d7b5eb..f731a7fa 100644 --- 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 987c0c76..522ac5bf 100644 --- 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 4816134f..60dc6e25 100644 --- 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 7689d365..03d8d040 100644 --- 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))