- A major fix in query.join(), when the "on" clause is an

attribute of an aliased() construct, but there is already
an existing join made out to a compatible target, query properly
joins to the right aliased() construct instead of sticking
onto the right side of the existing join.  [ticket:1706]

diff --git a/CHANGES b/CHANGES
index 9c22270bfa8ceb1a8168488fcb086114501b95a9..23e16e2e9726a05bc485e00f36a3515ea95e14e2 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -38,6 +38,12 @@ CHANGES
     one() can now also be called with a query that issued
     from_statement() to start with since it no longer modifies
     the query.  [ticket:1688]
+
+  - A major fix in query.join(), when the "on" clause is an
+    attribute of an aliased() construct, but there is already
+    an existing join made out to a compatible target, query properly
+    joins to the right aliased() construct instead of sticking
+    onto the right side of the existing join.  [ticket:1706]
     
   - Slight improvement to the fix for [ticket:1362] to not issue 
     needless updates of the primary key column during a so-called

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 5371e6eef6f26275823268c56e063ee2f4bfae16..a477ea54403b95d4d8845ed5f9068a40c64442f9 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -957,7 +957,9 @@ class Query(object):
         aliased, from_joinpoint = kwargs.pop('aliased', False), kwargs.pop('from_joinpoint', False)
         if kwargs:
             raise TypeError("unknown arguments: %s" % ','.join(kwargs.iterkeys()))
-        return self._join(props, outerjoin=False, create_aliases=aliased, from_joinpoint=from_joinpoint)
+        return self._join(props, 
+                            outerjoin=False, create_aliases=aliased, 
+                            from_joinpoint=from_joinpoint)
 
     @util.accepts_a_list_as_starargs(list_deprecation='deprecated')
     def outerjoin(self, *props, **kwargs):
@@ -970,7 +972,9 @@ class Query(object):
         aliased, from_joinpoint = kwargs.pop('aliased', False), kwargs.pop('from_joinpoint', False)
         if kwargs:
             raise TypeError("unknown arguments: %s" % ','.join(kwargs.iterkeys()))
-        return self._join(props, outerjoin=True, create_aliases=aliased, from_joinpoint=from_joinpoint)
+        return self._join(props, 
+                            outerjoin=True, create_aliases=aliased, 
+                            from_joinpoint=from_joinpoint)
 
     @_generative(_no_statement_condition, _no_limit_offset)
     def _join(self, keys, outerjoin, create_aliases, from_joinpoint):
@@ -1032,7 +1036,10 @@ class Query(object):
                 # TODO: no coverage here
                 raise NotImplementedError("query.join(a==b) not supported.")
             
-            self._join_left_to_right(left_entity, right_entity, onclause, outerjoin, create_aliases, prop)
+            self._join_left_to_right(
+                                left_entity, 
+                                right_entity, onclause, 
+                                outerjoin, create_aliases, prop)
 
     def _join_left_to_right(self, left, right, onclause, outerjoin, create_aliases, prop):
         """append a JOIN to the query's from clause."""
@@ -1045,7 +1052,8 @@ class Query(object):
 
         if right_mapper and prop and not right_mapper.common_parent(prop.mapper):
             raise sa_exc.InvalidRequestError(
-                "Join target %s does not correspond to the right side of join condition %s" % (right, onclause)
+                    "Join target %s does not correspond to "
+                    "the right side of join condition %s" % (right, onclause)
             )
 
         if not right_mapper and prop:
@@ -1126,17 +1134,22 @@ class Query(object):
                                 )
                             )
         
-        join_to_left = not is_aliased_class
+        join_to_left = not is_aliased_class and not left_is_aliased
 
         if self._from_obj:
-            replace_clause_index, clause = sql_util.find_join_source(self._from_obj, left_selectable)
+            replace_clause_index, clause = sql_util.find_join_source(
+                                                    self._from_obj, 
+                                                    left_selectable)
             if clause is not None:
                 # the entire query's FROM clause is an alias of itself (i.e. from_self(), similar).
                 # if the left clause is that one, ensure it aliases to the left side.
                 if self._from_obj_alias and clause is self._from_obj[0]:
                     join_to_left = True
 
-                clause = orm_join(clause, right, onclause, isouter=outerjoin, join_to_left=join_to_left)
+                clause = orm_join(clause, 
+                                    right, 
+                                    onclause, isouter=outerjoin, 
+                                    join_to_left=join_to_left)
 
                 self._from_obj = \
                         self._from_obj[:replace_clause_index] + \
@@ -1223,7 +1236,9 @@ class Query(object):
 
     @_generative(_no_statement_condition)
     def slice(self, start, stop):
-        """apply LIMIT/OFFSET to the ``Query`` based on a range and return the newly resulting ``Query``."""
+        """apply LIMIT/OFFSET to the ``Query`` based on a "
+        "range and return the newly resulting ``Query``."""
+        
         if start is not None and stop is not None:
             self._offset = (self._offset or 0) + start
             self._limit = stop - start

diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 6455d4d28ef736fd9d63d789c0ddd49991693fdd..51db26925ef98a46e9b68fcb1b67b6f454dfa63f 100644 (file)
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -1600,6 +1600,32 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
             , use_default_dialect=True
         )
         
+        # test #1 for [ticket:1706]
+        ualias = aliased(User)
+        self.assert_compile(
+            sess.query(ualias).
+                    join((oalias1, ualias.orders)).\
+                    join((Address, ualias.addresses)),
+            "SELECT users_1.id AS users_1_id, users_1.name AS "
+            "users_1_name FROM users AS users_1 JOIN orders AS orders_1 "
+            "ON users_1.id = orders_1.user_id JOIN addresses ON users_1.id "
+            "= addresses.user_id"
+            , use_default_dialect=True
+        )
+        
+        # test #2 for [ticket:1706]
+        ualias2 = aliased(User)
+        self.assert_compile(
+            sess.query(ualias).
+                    join((Address, ualias.addresses)).
+                    join((ualias2, Address.user)).
+                    join((Order, ualias.orders)),
+            "SELECT users_1.id AS users_1_id, users_1.name AS users_1_name FROM users "
+            "AS users_1 JOIN addresses ON users_1.id = addresses.user_id JOIN users AS users_2 "
+            "ON users_2.id = addresses.user_id JOIN orders ON users_1.id = orders.user_id"
+            , use_default_dialect=True
+        )
+        
     def test_overlapping_paths(self):
         for aliased in (True,False):
             # load a user who has an order that contains item id 3 and address id 1 (order 3, owned by jack)