diff options
| -rw-r--r-- | CHANGES | 7 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/query.py | 17 | ||||
| -rw-r--r-- | test/orm/query.py | 32 |
3 files changed, 50 insertions, 6 deletions
@@ -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 7a5721761..c5349e052 100644 --- 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 b446c1ae5..cba57914d 100644 --- 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() |