From b303eb9342a2bd697eda4c64a939249cd559ba67 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 28 Jul 2009 17:47:54 +0000 Subject: [PATCH] merged [ticket:1486] fix from 0.6 --- CHANGES | 5 ++++ lib/sqlalchemy/sql/expression.py | 15 ++++++++-- lib/sqlalchemy/sql/util.py | 2 +- test/sql/test_selectable.py | 51 +++++++++++++++++++++++++++----- 4 files changed, 61 insertions(+), 12 deletions(-) diff --git a/CHANGES b/CHANGES index eeba6f8bff..e79049b77e 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 0dae6716d4..83897ef051 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 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 a172eb4523..b0501c9134 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() -- 2.47.3