From e9175a37ce0b0b0e87ad728c8a6a10bed100065b Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 20 Oct 2016 17:47:19 -0400 Subject: [PATCH] Set autoincrement to False when modifying to non-Integer datatype Starting in SQLAlchemy 1.1, the rules for when "autoincrement=True" may be set on a column are more strict. The migrate tests are testing the alteration of a column from Integer to String and then regenerating; this means we need to set autoincrement to False as well. A related issue in SQLAlchemy 1.1 is also being fixed (see https://bitbucket.org/zzzeek/sqlalchemy/issues/3835/), however this fix is not needed in order for the tests to pass here. Change-Id: Ibd3a75fff13312411df87e17b6e5764865d69728 --- migrate/changeset/schema.py | 10 ++++++++-- migrate/tests/changeset/test_changeset.py | 13 +++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/migrate/changeset/schema.py b/migrate/changeset/schema.py index a0e42cc..931ef7b 100644 --- a/migrate/changeset/schema.py +++ b/migrate/changeset/schema.py @@ -353,8 +353,14 @@ class ColumnDelta(six.with_metaclass(MyMeta, DictMixin, sqlalchemy.schema.Schema self.process_column(self.result_column) # create an instance of class type if not yet - if 'type' in diffs and callable(self.result_column.type): - self.result_column.type = self.result_column.type() + if 'type' in diffs: + if callable(self.result_column.type): + self.result_column.type = self.result_column.type() + if self.result_column.autoincrement and \ + not issubclass( + self.result_column.type._type_affinity, + sqlalchemy.Integer): + self.result_column.autoincrement = False # add column to the table if self.table is not None and self.alter_metadata: diff --git a/migrate/tests/changeset/test_changeset.py b/migrate/tests/changeset/test_changeset.py index 57d0380..93e9965 100644 --- a/migrate/tests/changeset/test_changeset.py +++ b/migrate/tests/changeset/test_changeset.py @@ -687,12 +687,25 @@ class TestColumnChange(fixture.DB): self.assertTrue(isinstance(self.table.c.id.type, Integer)) self.assertEqual(self.table.c.id.nullable, False) + # SQLAlchemy 1.1 adds a third state to "autoincrement" called + # "auto". + self.assertTrue(self.table.c.id.autoincrement in ('auto', True)) + if not self.engine.name == 'firebird': self.table.c.id.alter(type=String(20)) self.assertEqual(self.table.c.id.nullable, False) + + # a rule makes sure that autoincrement is set to False + # when we change off of Integer + self.assertEqual(self.table.c.id.autoincrement, False) self.refresh_table(self.table.name) self.assertTrue(isinstance(self.table.c.id.type, String)) + # note that after reflection, "autoincrement" is likely + # to change back to a database-generated value. Should be + # False or "auto". if True, it's a bug; at least one of these + # exists prior to SQLAlchemy 1.1.3 + @fixture.usedb() def test_default(self): """Can change a column's server_default value (DefaultClauses only)