From: Mike Bayer Date: Thu, 5 Aug 2010 01:30:53 +0000 (-0400) Subject: - Fixed bug whereby replacing composite foreign key X-Git-Tag: rel_0_6_4~54 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=83c9b0dcdcae0e517a3055eca218e6eda61fd1fe;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - Fixed bug whereby replacing composite foreign key columns in a reflected table would cause an attempt to remove the reflected constraint from the table a second time, raising a KeyError. [ticket:1865] - fixed test of error message now that we've improved it (didn't know that msg had an assertion) --- diff --git a/CHANGES b/CHANGES index 484f8c6d74..c0b814a456 100644 --- a/CHANGES +++ b/CHANGES @@ -99,6 +99,11 @@ CHANGES - Added full description of parent table/column, target table/column in error message raised when ForeignKey can't resolve target. + + - Fixed bug whereby replacing composite foreign key + columns in a reflected table would cause an attempt + to remove the reflected constraint from the table + a second time, raising a KeyError. [ticket:1865] - declarative - if @classproperty is used with a regular class-bound diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py index 7bc3f63b76..5f82a02009 100644 --- a/lib/sqlalchemy/schema.py +++ b/lib/sqlalchemy/schema.py @@ -806,7 +806,11 @@ class Column(SchemaItem, expression.ColumnClause): for fk in col.foreign_keys: col.foreign_keys.remove(fk) table.foreign_keys.remove(fk) - table.constraints.remove(fk.constraint) + if fk.constraint in table.constraints: + # this might have been removed + # already, if it's a composite constraint + # and more than one col being replaced + table.constraints.remove(fk.constraint) table._columns.replace(self) diff --git a/test/engine/test_reflection.py b/test/engine/test_reflection.py index 1cbc9ab598..a82f1ec528 100644 --- a/test/engine/test_reflection.py +++ b/test/engine/test_reflection.py @@ -326,7 +326,44 @@ class ReflectionTest(TestBase, ComparesTables): assert len(a4.constraints) == 2 finally: meta.drop_all() + + @testing.provide_metadata + def test_override_composite_fk(self): + """Test double-remove of composite foreign key, when replaced.""" + + a = Table('a', + metadata, + Column('x', sa.Integer, primary_key=True), + Column('y', sa.Integer, primary_key=True), + ) + + b = Table('b', + metadata, + Column('x', sa.Integer, primary_key=True), + Column('y', sa.Integer, primary_key=True), + sa.ForeignKeyConstraint(['x', 'y'], ['a.x', 'a.y']) + ) + + metadata.create_all() + meta2 = MetaData() + + c1 = Column('x', sa.Integer, primary_key=True) + c2 = Column('y', sa.Integer, primary_key=True) + f1 = sa.ForeignKeyConstraint(['x', 'y'], ['a.x', 'a.y']) + b1 = Table('b', + meta2, c1, c2, f1, + autoload=True, + autoload_with=testing.db + ) + + assert b1.c.x is c1 + assert b1.c.y is c2 + assert f1 in b1.constraints + assert len(b1.constraints) == 2 + + + def test_override_keys(self): """test that columns can be overridden with a 'key', and that ForeignKey targeting during reflection still works.""" @@ -510,8 +547,9 @@ class ReflectionTest(TestBase, ComparesTables): ) assert_raises_message(sa.exc.InvalidRequestError, - "Could not find table 'pkgs' with which " - "to generate a foreign key", + "Foreign key assocated with column 'slots.pkg_id' " + "could not find table 'pkgs' with which to generate " + "a foreign key to target column 'pkg_id'", metadata.create_all) def test_composite_pks(self):