- query.join() raises an error when the target of the join

doesn't match the property-based attribute - while it's
unlikely anyone is doing this, the SQLAlchemy author was
guilty of this particular loosey-goosey behavior.

diff --git a/CHANGES b/CHANGES
index c618c6f1f737a4d0ada56a7ef41e2a5ec6a1b210..deed232a69fc32c949b49e7e349e370fece7a1e3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -86,7 +86,12 @@ CHANGES
       inheriting target, when used in the context of 
       prop.of_type(..).any()/has(), as well as 
       query.join(prop.of_type(...)).
-      
+    
+    - query.join() raises an error when the target of the join
+      doesn't match the property-based attribute - while it's 
+      unlikely anyone is doing this, the SQLAlchemy author was
+      guilty of this particular loosey-goosey behavior.
+        
     - Fixed bug when using weak_instance_map=False where modified
       events would not be intercepted for a flush(). [ticket:1272]
       

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 7a5721761e0b47929cd6c7c0825b9377fdd7ee48..c5349e052e2cd573f450393d75602163be5bcfc0 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -784,11 +784,10 @@ class Query(object):
         right_entity = None
 
         for arg1 in util.to_list(keys):
-            prop =  None
             aliased_entity = False
             alias_criterion = False
             left_entity = right_entity
-            right_entity = right_mapper = None
+            prop = of_type = right_entity = right_mapper = None
 
             if isinstance(arg1, tuple):
                 arg1, arg2 = arg1
@@ -827,6 +826,7 @@ class Query(object):
 
                 descriptor, prop = _entity_descriptor(left_entity, onclause)
                 right_mapper = prop.mapper
+
                 if not right_entity:
                     right_entity = right_mapper
             elif onclause is None:
@@ -849,7 +849,12 @@ class Query(object):
                 raise sa_exc.InvalidRequestError("Could not find a FROM clause to join from")
 
             mp, right_selectable, is_aliased_class = _entity_info(right_entity)
-
+            
+            if mp is not None and right_mapper is not None and not mp.common_parent(right_mapper):
+                raise sa_exc.InvalidRequestError(
+                    "Join target %s does not correspond to the right side of join condition %s" % (right_entity, onclause)
+                )
+            
             if not right_mapper and mp:
                 right_mapper = mp
 
@@ -886,8 +891,10 @@ class Query(object):
                     if prop.secondary:
                         self.__currenttables.add(prop.secondary)
                     self.__currenttables.add(prop.table)
-
-                    if not right_entity:
+                    
+                    if of_type:
+                        right_entity = of_type
+                    else:
                         right_entity = prop.mapper
 
             if alias_criterion:

diff --git a/test/orm/query.py b/test/orm/query.py
index b446c1ae5afa1fbb2eb7d5d3487db7dcbf297190..cba57914d1a4ac597bb3739a70b316d41f8dd144 100644 (file)
--- a/test/orm/query.py
+++ b/test/orm/query.py
@@ -979,6 +979,38 @@ class JoinTest(QueryTest):
             []
         )
     
+    def test_backwards_join(self):
+        # a more controversial feature.  join from
+        # User->Address, but the onclause is Address.user.
+        
+        sess = create_session()
+
+        self.assertEquals(
+            sess.query(User).join(Address.user).filter(Address.email_address=='ed@wood.com').all(),
+            [User(id=8,name=u'ed')]
+        )
+
+        # its actually not so controversial if you view it in terms
+        # of multiple entities.
+        self.assertEquals(
+            sess.query(User, Address).join(Address.user).filter(Address.email_address=='ed@wood.com').all(),
+            [(User(id=8,name=u'ed'), Address(email_address='ed@wood.com'))]
+        )
+        
+        # this was the controversial part.  now, raise an error if the feature is abused.
+        # before the error raise was added, this would silently work.....
+        self.assertRaises(
+            sa_exc.InvalidRequestError,
+            sess.query(User).join, (Address, Address.user),
+        )
+
+        # but this one would silently fail 
+        adalias = aliased(Address)
+        self.assertRaises(
+            sa_exc.InvalidRequestError,
+            sess.query(User).join, (adalias, Address.user),
+        )
+        
     def test_multiple_with_aliases(self):
         sess = create_session()