From 00dae5f77e51c8cb4902939a5b85efdfb920c68b Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 10 Jan 2015 17:08:03 -0500 Subject: [PATCH] - The rendering of a :class:`~sqlalchemy.schema.ForeignKeyConstraint` will now ensure that the names of the source and target columns are the database-side name of each column, and not the value of the ``.key`` attribute as may be set only on the Python side. This is because Alembic generates the DDL for constraints as standalone objects without the need to actually refer to an in-Python :class:`~sqlalchemy.schema.Table` object, so there's no step that would resolve these Python-only key names to database column names. fixes #259 --- alembic/autogenerate/render.py | 24 +++++++++---- docs/build/changelog.rst | 13 ++++++++ tests/test_autogen_render.py | 61 ++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 7 deletions(-) diff --git a/alembic/autogenerate/render.py b/alembic/autogenerate/render.py index caa36d9..13830cf 100644 --- a/alembic/autogenerate/render.py +++ b/alembic/autogenerate/render.py @@ -543,12 +543,22 @@ def _fk_colspec(fk, metadata_schema): """ colspec = fk._get_colspec() - if metadata_schema is not None and colspec.count(".") == 1: - # need to render schema breaking up tokens by hand, since the - # ForeignKeyConstraint here may not actually have a remote - # Table present - # no schema in the colspec, render it - colspec = "%s.%s" % (metadata_schema, colspec) + tokens = colspec.split(".") + tname, colname = tokens[-2:] + + if metadata_schema is not None and len(tokens) == 2: + table_fullname = "%s.%s" % (metadata_schema, tname) + else: + table_fullname = ".".join(tokens[0:-1]) + + if fk.parent is not None and fk.parent.table is not None: + # try to resolve the remote table and adjust for column.key + parent_metadata = fk.parent.table.metadata + if table_fullname in parent_metadata.tables: + colname = _ident(parent_metadata.tables[table_fullname].c[colname].name) + + colspec = "%s.%s" % (table_fullname, colname) + return colspec @@ -577,7 +587,7 @@ def _render_foreign_key(constraint, autogen_context): "[%(refcols)s], %(args)s)" % { "prefix": _sqlalchemy_autogenerate_prefix(autogen_context), "cols": ", ".join( - "%r" % f.parent.key for f in constraint.elements), + "%r" % _ident(f.parent.name) for f in constraint.elements), "refcols": ", ".join(repr(_fk_colspec(f, apply_metadata_schema)) for f in constraint.elements), "args": ", ".join( diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst index 6efc825..ba3d1ef 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: 259 + + The rendering of a :class:`~sqlalchemy.schema.ForeignKeyConstraint` + will now ensure that the names of the source and target columns are + the database-side name of each column, and not the value of the + ``.key`` attribute as may be set only on the Python side. + This is because Alembic generates the DDL for constraints + as standalone objects without the need to actually refer to an in-Python + :class:`~sqlalchemy.schema.Table` object, so there's no step that + would resolve these Python-only key names to database column names. + .. change:: :tags: bug, autogenerate :tickets: 260 diff --git a/tests/test_autogen_render.py b/tests/test_autogen_render.py index b3e8c4c..5313891 100644 --- a/tests/test_autogen_render.py +++ b/tests/test_autogen_render.py @@ -19,6 +19,9 @@ from alembic.testing.mock import patch from alembic import autogenerate, util, compat from alembic.testing import eq_, eq_ignore_whitespace, config +from alembic.testing.fixtures import op_fixture +from alembic import op # noqa +import sqlalchemy as sa # noqa py3k = sys.version_info >= (3, ) @@ -266,6 +269,64 @@ unique=False, """ "op.create_foreign_key('fk_a_id', 'b', 'a', ['a_id'], ['id'])" ) + def test_add_fk_constraint_inline_colkeys(self): + m = MetaData() + Table('a', m, Column('id', Integer, key='aid', primary_key=True)) + b = Table( + 'b', m, + Column('a_id', Integer, ForeignKey('a.aid'), key='baid')) + + py_code = autogenerate.render._add_table(b, self.autogen_context) + + eq_ignore_whitespace( + py_code, + "op.create_table('b'," + "sa.Column('a_id', sa.Integer(), nullable=True)," + "sa.ForeignKeyConstraint(['a_id'], ['a.id'], ))" + ) + + context = op_fixture() + eval(py_code) + context.assert_( + "CREATE TABLE b (a_id INTEGER, " + "FOREIGN KEY(a_id) REFERENCES a (id))") + + def test_add_fk_constraint_separate_colkeys(self): + m = MetaData() + Table('a', m, Column('id', Integer, key='aid', primary_key=True)) + b = Table('b', m, Column('a_id', Integer, key='baid')) + fk = ForeignKeyConstraint(['baid'], ['a.aid'], name='fk_a_id') + b.append_constraint(fk) + + py_code = autogenerate.render._add_table(b, self.autogen_context) + + eq_ignore_whitespace( + py_code, + "op.create_table('b'," + "sa.Column('a_id', sa.Integer(), nullable=True)," + "sa.ForeignKeyConstraint(['a_id'], ['a.id'], name='fk_a_id'))" + ) + + context = op_fixture() + eval(py_code) + context.assert_( + "CREATE TABLE b (a_id INTEGER, CONSTRAINT " + "fk_a_id FOREIGN KEY(a_id) REFERENCES a (id))") + + context = op_fixture() + py_code = autogenerate.render._add_fk_constraint( + fk, self.autogen_context) + + eq_ignore_whitespace( + autogenerate.render._add_fk_constraint(fk, self.autogen_context), + "op.create_foreign_key('fk_a_id', 'b', 'a', ['a_id'], ['id'])" + ) + + eval(py_code) + context.assert_( + "ALTER TABLE b ADD CONSTRAINT fk_a_id " + "FOREIGN KEY(a_id) REFERENCES a (id)") + def test_add_fk_constraint_schema(self): m = MetaData() Table(