- 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 484f8c6d74526950015f8bb2df83e911aef18fd1..c0b814a4569f5f63fb8dd47c2cde678851f3a002 100644 (file)
--- 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 7bc3f63b76f0721ac5ddb6325655010833a0d1ab..5f82a020097dd1f3bc08c757593d3ce456bc128f 100644 (file)
--- 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 1cbc9ab5983387a53b18c5cabe7711bc370525c4..a82f1ec5282a13fff1db4f437465aaa5a6353f2c 100644 (file)
--- 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):