From: Mike Bayer Date: Wed, 24 Feb 2010 00:43:09 +0000 (+0000) Subject: - A major fix in query.join(), when the "on" clause is an X-Git-Tag: rel_0_6beta2~130 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=930349073341ee3a91cc385f9151d2dde07254c6;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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 9c22270bfa..23e16e2e97 100644 --- 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 5371e6eef6..a477ea5440 100644 --- 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 6455d4d28e..51db26925e 100644 --- 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)