From d12da7c187c7121870216526fb89c782be5dd8fd Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 10 Jan 2015 16:32:17 -0500 Subject: [PATCH] - Fixed bug in foreign key autogenerate where if the in-Python table used custom column keys (e.g. using the ``key='foo'`` kwarg to ``Column``), the comparison of existing foreign keys to those specified in the metadata would fail, as the reflected table would not have these keys available which to match up. Foreign key comparison for autogenerate now ensures it's looking at the database-side names of the columns in all cases; this matches the same functionality within unique constraints and indexes. fixes #260 --- alembic/ddl/base.py | 5 ++- docs/build/changelog.rst | 13 +++++++ tests/test_autogen_fks.py | 72 +++++++++++++++++++++++++++++++++++ tests/test_autogen_indexes.py | 34 +++++++++++++++++ 4 files changed, 122 insertions(+), 2 deletions(-) diff --git a/alembic/ddl/base.py b/alembic/ddl/base.py index d497253c..dbdc991a 100644 --- a/alembic/ddl/base.py +++ b/alembic/ddl/base.py @@ -172,10 +172,11 @@ def _columns_for_constraint(constraint): def _fk_spec(constraint): if util.sqla_100: - source_columns = constraint.column_keys + source_columns = [ + constraint.columns[key].name for key in constraint.column_keys] else: source_columns = [ - element.parent.key for element in constraint.elements] + element.parent.name for element in constraint.elements] source_table = constraint.parent.name source_schema = constraint.parent.schema diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst index 63567098..6efc825d 100644 --- a/docs/build/changelog.rst +++ b/docs/build/changelog.rst @@ -6,6 +6,19 @@ Changelog .. changelog:: :version: 0.7.4 + .. change:: + :tags: bug, autogenerate + :tickets: 260 + + Fixed bug in foreign key autogenerate where if the in-Python table + used custom column keys (e.g. using the ``key='foo'`` kwarg to + ``Column``), the comparison of existing foreign keys to those specified + in the metadata would fail, as the reflected table would not have + these keys available which to match up. Foreign key comparison for + autogenerate now ensures it's looking at the database-side names + of the columns in all cases; this matches the same functionality + within unique constraints and indexes. + .. change:: :tags: bug, autogenerate :tickets: 261 diff --git a/tests/test_autogen_fks.py b/tests/test_autogen_fks.py index 36326c78..90d25c42 100644 --- a/tests/test_autogen_fks.py +++ b/tests/test_autogen_fks.py @@ -244,6 +244,78 @@ class AutogenerateForeignKeysTest(AutogenFixtureTest, TestBase): conditional_name="fk_test_name" ) + def test_add_fk_colkeys(self): + m1 = MetaData() + m2 = MetaData() + + Table('table', m1, + Column('id_1', String(10), primary_key=True), + Column('id_2', String(10), primary_key=True), + mysql_engine='InnoDB') + + Table('user', m1, + Column('id', Integer, primary_key=True), + Column('other_id_1', String(10)), + Column('other_id_2', String(10)), + mysql_engine='InnoDB') + + Table('table', m2, + Column('id_1', String(10), key='tid1', primary_key=True), + Column('id_2', String(10), key='tid2', primary_key=True), + mysql_engine='InnoDB') + + Table('user', m2, + Column('id', Integer, primary_key=True), + Column('other_id_1', String(10), key='oid1'), + Column('other_id_2', String(10), key='oid2'), + ForeignKeyConstraint(['oid1', 'oid2'], + ['table.tid1', 'table.tid2'], + name='fk_test_name'), + mysql_engine='InnoDB') + + diffs = self._fixture(m1, m2) + + self._assert_fk_diff( + diffs[0], "add_fk", + "user", ['other_id_1', 'other_id_2'], + 'table', ['id_1', 'id_2'], + name="fk_test_name" + ) + + def test_no_change_colkeys(self): + m1 = MetaData() + m2 = MetaData() + + Table('table', m1, + Column('id_1', String(10), primary_key=True), + Column('id_2', String(10), primary_key=True), + mysql_engine='InnoDB') + + Table('user', m1, + Column('id', Integer, primary_key=True), + Column('other_id_1', String(10)), + Column('other_id_2', String(10)), + ForeignKeyConstraint(['other_id_1', 'other_id_2'], + ['table.id_1', 'table.id_2']), + mysql_engine='InnoDB') + + Table('table', m2, + Column('id_1', String(10), key='tid1', primary_key=True), + Column('id_2', String(10), key='tid2', primary_key=True), + mysql_engine='InnoDB') + + Table('user', m2, + Column('id', Integer, primary_key=True), + Column('other_id_1', String(10), key='oid1'), + Column('other_id_2', String(10), key='oid2'), + ForeignKeyConstraint(['oid1', 'oid2'], + ['table.tid1', 'table.tid2']), + mysql_engine='InnoDB') + + diffs = self._fixture(m1, m2) + + eq_(diffs, []) + class IncludeHooksTest(AutogenFixtureTest, TestBase): __backend__ = True diff --git a/tests/test_autogen_indexes.py b/tests/test_autogen_indexes.py index 95d43afd..702e2df1 100644 --- a/tests/test_autogen_indexes.py +++ b/tests/test_autogen_indexes.py @@ -258,6 +258,40 @@ class AutogenerateUniqueIndexTest(AutogenFixtureTest, TestBase): diffs = self._fixture(m1, m2) eq_(diffs, []) + def test_nothing_changed_unique_w_colkeys(self): + m1 = MetaData() + m2 = MetaData() + + Table('nothing_changed', m1, + Column('x', String(20), key='nx'), + UniqueConstraint('nx') + ) + + Table('nothing_changed', m2, + Column('x', String(20), key='nx'), + UniqueConstraint('nx') + ) + + diffs = self._fixture(m1, m2) + eq_(diffs, []) + + def test_nothing_changed_index_w_colkeys(self): + m1 = MetaData() + m2 = MetaData() + + Table('nothing_changed', m1, + Column('x', String(20), key='nx'), + Index('foobar', 'nx') + ) + + Table('nothing_changed', m2, + Column('x', String(20), key='nx'), + Index('foobar', 'nx') + ) + + diffs = self._fixture(m1, m2) + eq_(diffs, []) + def test_nothing_changed_index_named_as_column(self): m1 = MetaData() m2 = MetaData() -- 2.47.2