- 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 bf8a19f8a0da4db16367ced91f9a74d327f0cdaa..01348d468b6e6cf3706f866e4dc9ae14889ed363 100644 (file)
--- 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 c9a2dbe597dde558f5db95b7a20a0fadf827573c..dbf58946fe942ce18e52ac2647a9775659ebcbc3 100644 (file)
--- 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 a0b520f339c9cbe1aa406ccaa78209c78693c6e4..fa96798fe6959d56d39d06f874bb834de57e0365 100644 (file)
--- 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 e6496a401ac2ec8b596847291603c7a9d3f620a6..ad3c6432e9156382704c7e79474aece7fbc78ac2 100644 (file)
--- 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 c46825dc29bab799b4991427e6d744662491528b..52c0e37e6cd825c3a509892d59b3f7c7ff83e488 100644 (file)
--- 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 28b0e9da02664bb9c45a21a4d326a7edb8cc6096..340515039a62dbe1f2b3671414bb52edcb11c819 100644 (file)
--- 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 9958724e1383d376b25f3050c81cea0596d45512..8f00ea6acd4b1fe88185fce7affa76ead607b100 100644 (file)
--- 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