merged [ticket:1486] fix from 0.6

4 files changed, 61 insertions, 12 deletions

diff --git a/CHANGES b/CHANGES
index eeba6f8bf..e79049b77 100644
--- a/CHANGES
+++ b/CHANGES
@@ -45,6 +45,11 @@ CHANGES
       [ticket:1424].  See
       http://www.sqlalchemy.org/trac/wiki/UsageRecipes/PreFilteredQuery
       for an example.
+
+    - Fixed a somewhat hypothetical issue which would result
+      in the wrong primary key being calculated for a mapper
+      using the old polymorphic_union function - but this
+      is old stuff.  [ticket:1486]
       
 - sql
     - Fixed a bug in extract() introduced in 0.5.4 whereby
diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index 0dae6716d..83897ef05 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -3170,10 +3170,19 @@ class CompoundSelect(_SelectBaseMixin, FromClause):
 
     def _populate_column_collection(self):
         for cols in zip(*[s.c for s in self.selects]):
-            proxy = cols[0]._make_proxy(self, name=self.use_labels and cols[0]._label or None)
+            # this is a slightly hacky thing - the union exports a column that
+            # resembles just that of the *first* selectable.  to get at a "composite" column,
+            # particularly foreign keys, you have to dig through the proxies collection 
+            # which we generate below.  We may want to improve upon this,
+            # such as perhaps _make_proxy can accept a list of other columns that
+            # are "shared" - schema.column can then copy all the ForeignKeys in.
+            # this would allow the union() to have all those fks too.
+            proxy = cols[0]._make_proxy(
+                            self, name=self.use_labels and cols[0]._label or None)
             
-            # place a 'weight' annotation corresponding to how low in the list of select()s
-            # the column occurs, so that the corresponding_column() operation
+            # hand-construct the "proxies" collection to include all derived columns
+            # place a 'weight' annotation corresponding to how low in the list of
+            # select()s the column occurs, so that the corresponding_column() operation
             # can resolve conflicts
             proxy.proxies = [c._annotate({'weight':i + 1}) for i, c in enumerate(cols)]
             
diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index ac95c3a20..27ae3e624 100644
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -298,7 +298,7 @@ def reduce_columns(columns, *clauses, **kw):
 
     omit = util.column_set()
     for col in columns:
-        for fk in col.foreign_keys:
+        for fk in chain(*[c.foreign_keys for c in col.proxy_set]):
             for c in columns:
                 if c is col:
                     continue
diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index a172eb452..b0501c913 100644
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -5,7 +5,7 @@ from sqlalchemy import *
 from sqlalchemy.test import *
 from sqlalchemy.sql import util as sql_util, visitors
 from sqlalchemy import exc
-from sqlalchemy.sql import table, column
+from sqlalchemy.sql import table, column, null
 from sqlalchemy import util
 
 metadata = MetaData()
@@ -416,19 +416,54 @@ class ReduceTest(TestBase, AssertsExecutionResults):
             Column('magazine_page_id', Integer, ForeignKey('magazine_page.page_id'), primary_key=True),
         )
         
-        from sqlalchemy.orm.util import polymorphic_union
-        pjoin = polymorphic_union(
-            {
-                'm': page_table.join(magazine_page_table),
-                'c': page_table.join(magazine_page_table).join(classified_page_table),
-            }, None, 'page_join')
+       # this is essentially the union formed by the ORM's polymorphic_union function.
+        # we define two versions with different ordering of selects.
+
+        # the first selectable has the "real" column classified_page.magazine_page_id
+        pjoin = union(
+            select([
+                page_table.c.id, 
+                magazine_page_table.c.page_id, 
+                classified_page_table.c.magazine_page_id
+            ]).select_from(page_table.join(magazine_page_table).join(classified_page_table)),
+
+            select([
+                page_table.c.id, 
+                magazine_page_table.c.page_id, 
+                cast(null(), Integer).label('magazine_page_id')
+            ]).select_from(page_table.join(magazine_page_table)),
             
+        ).alias('pjoin')
+
         eq_(
             util.column_set(sql_util.reduce_columns([pjoin.c.id, pjoin.c.page_id, pjoin.c.magazine_page_id])),
             util.column_set([pjoin.c.id])
         )    
-    
+
+        # the first selectable has a CAST, which is a placeholder for
+        # classified_page.magazine_page_id in the second selectable.  reduce_columns
+        # needs to take into account all foreign keys derived from pjoin.c.magazine_page_id.
+        # the UNION construct currently makes the external column look like that of the first
+        # selectable only.
+        pjoin = union(
+            select([
+                page_table.c.id, 
+                magazine_page_table.c.page_id, 
+                cast(null(), Integer).label('magazine_page_id')
+            ]).select_from(page_table.join(magazine_page_table)),
             
+            select([
+                page_table.c.id, 
+                magazine_page_table.c.page_id, 
+                classified_page_table.c.magazine_page_id
+            ]).select_from(page_table.join(magazine_page_table).join(classified_page_table))
+        ).alias('pjoin')
+
+        eq_(
+            util.column_set(sql_util.reduce_columns([pjoin.c.id, pjoin.c.page_id, pjoin.c.magazine_page_id])),
+            util.column_set([pjoin.c.id])
+        )    
+
 class DerivedTest(TestBase, AssertsExecutionResults):
     def test_table(self):
         meta = MetaData()