- 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

7 files changed, 62 insertions, 14 deletions

diff --git a/CHANGES b/CHANGES
index bf8a19f8a..01348d468 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 c9a2dbe59..dbf58946f 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 a0b520f33..fa96798fe 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 e6496a401..ad3c6432e 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 c46825dc2..52c0e37e6 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 28b0e9da0..340515039 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 9958724e1..8f00ea6ac 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