From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 28 Jul 2009 17:13:31 +0000 (+0000)
Subject: improved reduce_columns() to navigate around a CompositeSelect that buries foreign... 
X-Git-Tag: rel_0_6_6~59
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b3b0859e2933b1ecbe4a00ab37ee8baceab11239;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

improved reduce_columns() to navigate around a CompositeSelect that buries foreign-key holding columns down
on later selectables, [ticket:1486]
---

diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index 2da5184e69..91e0e74ae4 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -3321,9 +3321,17 @@ class CompoundSelect(_SelectBaseMixin, FromClause):
 
     def _populate_column_collection(self):
         for cols in zip(*[s.c for s in self.selects]):
+            # 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)
             
+            # 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
diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index ac95c3a209..27ae3e6247 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 670ae1fd0c..0e0cef38f2 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()