- use ForeignKey.column as _colspec source in Column._make_proxy(), preventing needless

redundant string arithmetic in memoized ForeignKey.column method
- _pre_existing_column attribute becomes optional, only needed for original Table-bound column, not proxies
- compare two ForeignKeys based on target_fullname, don't assume self._colspec is a string
- Fixed bug when overriding a Column with a ForeignKey
on a reflected table, where derived columns (i.e. the
"virtual" columns of a select, etc.) would inadvertently
call upon schema-level cleanup logic intended only
for the original column. [ticket:1278]

diff --git a/CHANGES b/CHANGES
index 56f2e72ab8db6507cb1be2c9c45d922eb2bed427..c45dd101b850f01436b6500d1c07a8713044196a 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -61,6 +61,12 @@ CHANGES
       NoneType error when it's string output is requsted
       (such as in a stack trace).
       
+    - Fixed bug when overriding a Column with a ForeignKey
+      on a reflected table, where derived columns (i.e. the 
+      "virtual" columns of a select, etc.) would inadvertently
+      call upon schema-level cleanup logic intended only
+      for the original column. [ticket:1278]
+      
 - declarative
     - Can now specify Column objects on subclasses which have no
       table of their own (i.e. use single table inheritance).  

diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index d454bc7cff322fde73e1e84bc7443ab044532e7f..c4f9a2895647e66904632aa5432feb940ae8b115 100644 (file)
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -666,7 +666,9 @@ class Column(SchemaItem, expression.ColumnClause):
         if getattr(self, 'table', None) is not None:
             raise exc.ArgumentError("this Column already has a table!")
 
-        self._pre_existing_column = table._columns.get(self.key)
+        if self.key in table._columns:
+            # note the column being replaced, if any
+            self._pre_existing_column = table._columns.get(self.key)
         table._columns.replace(self)
 
         if self.primary_key:
@@ -734,11 +736,17 @@ class Column(SchemaItem, expression.ColumnClause):
         (such as an alias or select statement).
 
         """
-        fk = [ForeignKey(f._colspec) for f in self.foreign_keys]
-        c = Column(name or self.name, self.type, self.default, key = name or self.key, primary_key = self.primary_key, nullable = self.nullable, quote=self.quote, *fk)
+        fk = [ForeignKey(f.column) for f in self.foreign_keys]
+        c = Column(
+            name or self.name, 
+            self.type, 
+            self.default, 
+            key = name or self.key, 
+            primary_key = self.primary_key, 
+            nullable = self.nullable, 
+            quote=self.quote, *fk)
         c.table = selectable
         c.proxies = [self]
-        c._pre_existing_column = self._pre_existing_column
         selectable.columns.add(c)
         if self.primary_key:
             selectable.primary_key.add(c)
@@ -924,10 +932,10 @@ class ForeignKey(SchemaItem):
             raise exc.InvalidRequestError("This ForeignKey already has a parent !")
         self.parent = column
 
-        if self.parent._pre_existing_column is not None:
+        if hasattr(self.parent, '_pre_existing_column'):
             # remove existing FK which matches us
             for fk in self.parent._pre_existing_column.foreign_keys:
-                if fk._colspec == self._colspec:
+                if fk.target_fullname == self.target_fullname:
                     self.parent.table.foreign_keys.remove(fk)
                     self.parent.table.constraints.remove(fk.constraint)
 

diff --git a/test/engine/reflection.py b/test/engine/reflection.py
index 8e6a3df987c73f4426b2832861456ae916d63b0a..d8412237fbdc1b53c1d110ef8010ebc2b30a7b8b 100644 (file)
--- a/test/engine/reflection.py
+++ b/test/engine/reflection.py
@@ -311,6 +311,8 @@ class ReflectionTest(TestBase, ComparesTables):
                 autoload=True)
             u2 = Table('users', meta2, autoload=True)
 
+            s = sa.select([a2])
+            assert s.c.user_id
             assert len(a2.foreign_keys) == 1
             assert len(a2.c.user_id.foreign_keys) == 1
             assert len(a2.constraints) == 2
@@ -328,6 +330,8 @@ class ReflectionTest(TestBase, ComparesTables):
                 Column('user_id', sa.Integer, sa.ForeignKey('users.id')),
                 autoload=True)
 
+            s = sa.select([a2])
+            assert s.c.user_id
             assert len(a2.foreign_keys) == 1
             assert len(a2.c.user_id.foreign_keys) == 1
             assert len(a2.constraints) == 2