- Patched a case where query.join() would adapt the

right side to the right side of the left's join
inappropriately [ticket:1925]

2 files changed, 39 insertions, 6 deletions

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index b22a10b55..0ce84435f 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -1250,7 +1250,7 @@ class Query(object):
                         (left, right))
             
         left_mapper, left_selectable, left_is_aliased = _entity_info(left)
-        right_mapper, right_selectable, is_aliased_class = _entity_info(right)
+        right_mapper, right_selectable, right_is_aliased = _entity_info(right)
 
         if right_mapper and prop and \
                 not right_mapper.common_parent(prop.mapper):
@@ -1279,7 +1279,7 @@ class Query(object):
             need_adapter = True
 
         aliased_entity = right_mapper and \
-                            not is_aliased_class and \
+                            not right_is_aliased and \
                             (
                                 right_mapper.with_polymorphic or
                                 isinstance(
@@ -1342,8 +1342,16 @@ class Query(object):
                         )
                     )
         
-        join_to_left = not is_aliased_class and not left_is_aliased
-
+        # this is an overly broad assumption here, but there's a 
+        # very wide variety of situations where we rely upon orm.join's
+        # adaption to glue clauses together, with joined-table inheritance's
+        # wide array of variables taking up most of the space.
+        # Setting the flag here is still a guess, so it is a bug
+        # that we don't have definitive criterion to determine when 
+        # adaption should be enabled (or perhaps that we're even doing the 
+        # whole thing the way we are here).
+        join_to_left = not right_is_aliased and not left_is_aliased
+            
         if self._from_obj and left_selectable is not None:
             replace_clause_index, clause = sql_util.find_join_source(
                                                     self._from_obj, 
@@ -1351,10 +1359,16 @@ class Query(object):
             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.
+                # ensure it adapts to the left side.
                 if self._from_obj_alias and clause is self._from_obj[0]:
                     join_to_left = True
-
+                
+                # An exception case where adaption to the left edge is not
+                # desirable.  See above note on join_to_left.
+                if join_to_left and isinstance(clause, expression.Join) and \
+                    sql_util.clause_is_present(left_selectable, clause):
+                    join_to_left = False
+                    
                 clause = orm_join(clause, 
                                     right, 
                                     onclause, isouter=outerjoin, 
diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index bd4f70247..638549e12 100644
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -92,6 +92,25 @@ def find_columns(clause):
     visitors.traverse(clause, {}, {'column':cols.add})
     return cols
 
+def clause_is_present(clause, search):
+    """Given a target clause and a second to search within, return True
+    if the target is plainly present in the search without any
+    subqueries or aliases involved.
+    
+    Basically descends through Joins.
+    
+    """
+
+    stack = [search]
+    while stack:
+        elem = stack.pop()
+        if clause is elem:
+            return True
+        elif isinstance(elem, expression.Join):
+            stack.extend((elem.left, elem.right))
+    return False
+    
+    
 def bind_values(clause):
     """Return an ordered list of "bound" values in the given clause.