From: Mike Bayer Date: Wed, 14 Mar 2007 23:48:07 +0000 (+0000) Subject: - eager loading will not "aliasize" "order by" clauses that were placed X-Git-Tag: rel_0_3_6~25 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a5bf257126f709b6d6cf55ce5b38e209f09d689c;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - eager loading will not "aliasize" "order by" clauses that were placed in the select statement by something other than the eager loader itself, to fix possibility of dupe columns as illustrated in [ticket:495]. however, this means you have to be more careful with the columns placed in the "order by" of Query.select(), that you have explicitly named them in your criterion (i.e. you cant rely on the eager loader adding them in for you) - query._join_to (which powers join, join_via, etc) properly takes secondary table into account when constructing joins --- diff --git a/CHANGES b/CHANGES index bf8a19f8a0..01348d468b 100644 --- a/CHANGES +++ b/CHANGES @@ -89,6 +89,14 @@ for constructing reasonable join conditions (otherwise you can get full cartesian products). result set is the list of tuples, non-uniqued. + - eager loading will not "aliasize" "order by" clauses that were placed + in the select statement by something other than the eager loader + itself, to fix possibility of dupe columns as illustrated in + [ticket:495]. however, this means you have to be more careful with + the columns placed in the "order by" of Query.select(), that you have + explicitly named them in your criterion (i.e. you cant rely on the + eager loader adding them in for you) + - strings and columns can also be sent to the *args of instances() where those exact result columns will be part of the result tuples. diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index c9a2dbe597..dbf58946fe 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -400,9 +400,9 @@ class PropertyLoader(StrategizedProperty): def _is_self_referential(self): return self.parent.mapped_table is self.target or self.parent.select_table is self.target - def get_join(self, parent): + def get_join(self, parent, primary=True, secondary=True): try: - return self._parent_join_cache[parent] + return self._parent_join_cache[(parent, primary, secondary)] except KeyError: parent_equivalents = parent._get_inherited_column_equivalents() primaryjoin = self.polymorphic_primaryjoin.copy_container() @@ -418,10 +418,15 @@ class PropertyLoader(StrategizedProperty): sql_util.ClauseAdapter(parent.select_table, exclude=self.foreign_keys, equivalents=parent_equivalents).traverse(primaryjoin) if secondaryjoin is not None: - j = primaryjoin & secondaryjoin + if secondary and not primary: + j = secondaryjoin + elif primary and secondary: + j = primaryjoin & secondaryjoin + elif primary and not secondary: + j = primaryjoin else: j = primaryjoin - self._parent_join_cache[parent] = j + self._parent_join_cache[(parent, primary, secondary)] = j return j def register_dependencies(self, uowcommit): diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index a0b520f339..fa96798fe6 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -408,9 +408,17 @@ class Query(object): for key in keys: prop = mapper.props[key] if outerjoin: - clause = clause.outerjoin(prop.select_table, prop.get_join(mapper)) + if prop.secondary: + clause = clause.outerjoin(prop.secondary, prop.get_join(mapper, primary=True, secondary=False)) + clause = clause.outerjoin(prop.select_table, prop.get_join(mapper, primary=False)) + else: + clause = clause.outerjoin(prop.select_table, prop.get_join(mapper)) else: - clause = clause.join(prop.select_table, prop.get_join(mapper)) + if prop.secondary: + clause = clause.join(prop.secondary, prop.get_join(mapper, primary=True, secondary=False)) + clause = clause.join(prop.select_table, prop.get_join(mapper, primary=False)) + else: + clause = clause.join(prop.select_table, prop.get_join(mapper)) mapper = prop.mapper return (clause, mapper) diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index e6496a401a..ad3c6432e9 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -472,8 +472,6 @@ class EagerLoader(AbstractRelationLoader): if clauses.eager_order_by: statement.order_by(*util.to_list(clauses.eager_order_by)) - elif getattr(statement, 'order_by_clause', None): - clauses._aliasize_orderby(statement.order_by_clause, False) statement.append_from(statement._outerjoin) for value in self.select_mapper.props.values(): diff --git a/test/orm/mapper.py b/test/orm/mapper.py index c46825dc29..52c0e37e6c 100644 --- a/test/orm/mapper.py +++ b/test/orm/mapper.py @@ -319,6 +319,21 @@ class MapperTest(MapperSuperTest): assert False except AttributeError: assert True + + def testjoinviam2m(self): + """test the join_via and join_to functions""" + m = mapper(Order, orders, properties = { + 'items' : relation(mapper(Item, orderitems, properties = { + 'keywords' : relation(mapper(Keyword, keywords), itemkeywords) + })) + }) + + sess = create_session() + q = sess.query(m) + + l = q.filter(keywords.c.name=='square').join(['items', 'keywords']).list() + self.assert_result(l, Order, order_result[1]) + def testcustomjoin(self): """test that the from_obj parameter to query.select() can be used @@ -1315,7 +1330,6 @@ class EagerTest(MapperSuperTest): def testmanytomany(self): items = orderitems - m = mapper(Item, items, properties = dict( keywords = relation(mapper(Keyword, keywords), itemkeywords, lazy=False, order_by=[keywords.c.keyword_id]), )) @@ -1328,6 +1342,7 @@ class EagerTest(MapperSuperTest): {'item_id' : 1, 'keywords' : (Keyword, [{'keyword_id' : 2}, {'keyword_id' : 4}, {'keyword_id' : 6}])}, {'item_id' : 2, 'keywords' : (Keyword, [{'keyword_id' : 2}, {'keyword_id' : 5}, {'keyword_id' : 7}])}, ) + def testmanytomanyoptions(self): items = orderitems diff --git a/test/orm/unitofwork.py b/test/orm/unitofwork.py index 28b0e9da02..340515039a 100644 --- a/test/orm/unitofwork.py +++ b/test/orm/unitofwork.py @@ -1255,7 +1255,7 @@ class ManyToManyTest(UnitOfWorkTest): keywordmapper = mapper(Keyword, keywords) m = mapper(Item, items, properties = dict( - keywords = relation(keywordmapper, itemkeywords, lazy = False), + keywords = relation(keywordmapper, itemkeywords, lazy = False, order_by=keywords.c.name), )) data = [Item, @@ -1289,7 +1289,7 @@ class ManyToManyTest(UnitOfWorkTest): ctx.current.flush() - l = ctx.current.query(m).select(items.c.item_name.in_(*[e['item_name'] for e in data[1:]]), order_by=[items.c.item_name, keywords.c.name]) + l = ctx.current.query(m).select(items.c.item_name.in_(*[e['item_name'] for e in data[1:]]), order_by=[items.c.item_name]) self.assert_result(l, *data) objects[4].item_name = 'item4updated' @@ -1408,7 +1408,7 @@ class ManyToManyTest(UnitOfWorkTest): # the reorganization of mapper construction affected this, but was fixed again m = mapper(Item, items, properties = dict( keywords = relation(mapper(IKAssociation, itemkeywords, properties = dict( - keyword = relation(mapper(Keyword, keywords, non_primary=True), lazy = False, uselist = False) + keyword = relation(mapper(Keyword, keywords, non_primary=True), lazy = False, uselist = False, order_by=keywords.c.name) ), primary_key = [itemkeywords.c.item_id, itemkeywords.c.keyword_id]), lazy = False) )) @@ -1455,7 +1455,7 @@ class ManyToManyTest(UnitOfWorkTest): ctx.current.flush() ctx.current.clear() - l = Query(m).select(items.c.item_name.in_(*[e['item_name'] for e in data[1:]]), order_by=[items.c.item_name, keywords.c.name]) + l = Query(m).select(items.c.item_name.in_(*[e['item_name'] for e in data[1:]]), order_by=[items.c.item_name]) self.assert_result(l, *data) def testm2mmultitable(self): diff --git a/test/tables.py b/test/tables.py index 9958724e13..8f00ea6acd 100644 --- a/test/tables.py +++ b/test/tables.py @@ -191,11 +191,25 @@ user_all_result = [ }] item_keyword_result = [ -{'item_id' : 1, 'keywords' : (Keyword, [{'keyword_id' : 2}, {'keyword_id' : 4}, {'keyword_id' : 6}])}, +{'item_id' : 1, 'keywords' : (Keyword, [{'keyword_id' : 2, 'name':'red'}, {'keyword_id' : 4, 'name':'big'}, {'keyword_id' : 6, 'name':'round'}])}, {'item_id' : 2, 'keywords' : (Keyword, [{'keyword_id' : 2, 'name':'red'}, {'keyword_id' : 5, 'name':'small'}, {'keyword_id' : 7, 'name':'square'}])}, {'item_id' : 3, 'keywords' : (Keyword, [{'keyword_id' : 3,'name':'green'}, {'keyword_id' : 4,'name':'big'}, {'keyword_id' : 6,'name':'round'}])}, {'item_id' : 4, 'keywords' : (Keyword, [])}, {'item_id' : 5, 'keywords' : (Keyword, [])} ] +order_result = [ +{'order_id' : 1, 'items':(Item, [])}, +{'order_id' : 2, 'items':(Item, [ + {'item_id' : 1, 'keywords' : (Keyword, [{'keyword_id' : 2}, {'keyword_id' : 4}, {'keyword_id' : 6}])}, + {'item_id' : 2, 'keywords' : (Keyword, [{'keyword_id' : 2, 'name':'red'}, {'keyword_id' : 5, 'name':'small'}, {'keyword_id' : 7, 'name':'square'}])}, + ])}, +{'order_id' : 3, 'items':(Item, [ + {'item_id' : 3, 'keywords' : (Keyword, [{'keyword_id' : 3,'name':'green'}, {'keyword_id' : 4,'name':'big'}, {'keyword_id' : 6,'name':'round'}])}, + {'item_id' : 4, 'keywords' : (Keyword, [])}, + {'item_id' : 5, 'keywords' : (Keyword, [])} + ])}, +{'order_id' : 4, 'items':(Item, [])}, +{'order_id' : 5, 'items':(Item, [])}, +] #db.echo = True