From dedc85ba9d8e3292311c4593d2252a44556cd7fe Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 31 Mar 2015 13:29:10 -0400 Subject: [PATCH] - further fixes for #3347; track the mappers we're joining between fully and match on those, rather than trying to compare selectables; fixes #3347 --- lib/sqlalchemy/orm/strategies.py | 45 +++++++++++++++++--------------- lib/sqlalchemy/orm/util.py | 30 ++++++++++++++++++++- test/orm/test_eager_relations.py | 41 +++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 22 deletions(-) diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 9aae8e5c82..c03e133dee 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -1338,18 +1338,19 @@ class JoinedLoader(AbstractRelationshipLoader): if attach_on_outside: # this is the "classic" eager join case. - eagerjoin = orm_util.join( + eagerjoin = orm_util._ORMJoin( towrap, clauses.aliased_class, onclause, isouter=not innerjoin or ( chained_from_outerjoin and isinstance(towrap, sql.Join) - ) + ), _left_memo=self.parent, _right_memo=self.mapper ) else: # all other cases are innerjoin=='nested' approach eagerjoin = self._splice_nested_inner_join( path, towrap, clauses, onclause) + context.eager_joins[entity_key] = eagerjoin # send a hint to the Query as to where it may "splice" this join @@ -1382,34 +1383,38 @@ class JoinedLoader(AbstractRelationshipLoader): def _splice_nested_inner_join( self, path, join_obj, clauses, onclause, splicing=False): - if not splicing: + if splicing is False: # first call is always handed a join object # from the outside - assert isinstance(join_obj, sql.Join) + assert isinstance(join_obj, orm_util._ORMJoin) elif isinstance(join_obj, sql.selectable.FromGrouping): return self._splice_nested_inner_join( - path, join_obj.element, clauses, onclause, True + path, join_obj.element, clauses, onclause, splicing ) - elif not isinstance(join_obj, sql.Join): - if join_obj.is_derived_from(path[-2].selectable): - return orm_util.join( + elif not isinstance(join_obj, orm_util._ORMJoin): + if path[-2] is splicing: + return orm_util._ORMJoin( join_obj, clauses.aliased_class, - onclause, isouter=False + onclause, isouter=False, + _left_memo=splicing, + _right_memo=path[-1].mapper ) else: # only here if splicing == True return None target_join = self._splice_nested_inner_join( - path, join_obj.right, clauses, onclause, True) + path, join_obj.right, clauses, + onclause, join_obj._right_memo) if target_join is None: right_splice = False target_join = self._splice_nested_inner_join( - path, join_obj.left, clauses, onclause, True) + path, join_obj.left, clauses, + onclause, join_obj._left_memo) if target_join is None: # should only return None when recursively called, # e.g. splicing==True - assert splicing, \ + assert splicing is not False, \ "assertion failed attempting to produce joined eager loads" return None else: @@ -1420,19 +1425,17 @@ class JoinedLoader(AbstractRelationshipLoader): # a JOIN b JOIN c JOIN .. to avoid needless # parenthesis nesting if not join_obj.isouter and not target_join.isouter: - eagerjoin = orm_util.join( - join_obj.left, target_join.left, - join_obj.onclause, isouter=False, - ).join(target_join.right, - target_join.onclause, isouter=False) + eagerjoin = join_obj._splice_into_center(target_join) else: - eagerjoin = orm_util.join( + eagerjoin = orm_util._ORMJoin( join_obj.left, target_join, - join_obj.onclause, isouter=join_obj.isouter) + join_obj.onclause, isouter=join_obj.isouter, + _left_memo=join_obj._left_memo) else: - eagerjoin = orm_util.join( + eagerjoin = orm_util._ORMJoin( target_join, join_obj.right, - join_obj.onclause, isouter=join_obj.isouter) + join_obj.onclause, isouter=join_obj.isouter, + _right_memo=join_obj._right_memo) eagerjoin._target_adapter = target_join._target_adapter return eagerjoin diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index ad071376dc..b3f3bc5fa7 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -776,7 +776,10 @@ class _ORMJoin(expression.Join): __visit_name__ = expression.Join.__visit_name__ - def __init__(self, left, right, onclause=None, isouter=False): + def __init__( + self, + left, right, onclause=None, isouter=False, + _left_memo=None, _right_memo=None): left_info = inspection.inspect(left) left_orm_info = getattr(left, '_joined_from_info', left_info) @@ -786,6 +789,9 @@ class _ORMJoin(expression.Join): self._joined_from_info = right_info + self._left_memo = _left_memo + self._right_memo = _right_memo + if isinstance(onclause, util.string_types): onclause = getattr(left_orm_info.entity, onclause) @@ -837,6 +843,28 @@ class _ORMJoin(expression.Join): single_crit = right_info._adapter.traverse(single_crit) self.onclause = self.onclause & single_crit + def _splice_into_center(self, other): + """Splice a join into the center. + + Given join(a, b) and join(b, c), return join(a, b).join(c) + + """ + assert self.right is other.left + + left = _ORMJoin( + self.left, other.left, + self.onclause, isouter=self.isouter, + _left_memo=self._left_memo, + _right_memo=other._left_memo + ) + + return _ORMJoin( + left, + other.right, + other.onclause, isouter=other.isouter, + _right_memo=other._right_memo + ) + def join(self, right, onclause=None, isouter=False, join_to_left=None): return _ORMJoin(self, right, onclause, isouter) diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index e085f44a60..278fb61a9c 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -2257,6 +2257,47 @@ class InnerJoinSplicingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): ) self._assert_result(q) + def test_splice_onto_np_mapper(self): + A = self.classes.A + B = self.classes.B + C1 = self.classes.C1 + b_table = self.tables.b + c1_table = self.tables.c1 + + from sqlalchemy import inspect + + weird_selectable = b_table.outerjoin(c1_table) + + b_np = mapper(B, weird_selectable, non_primary=True, properties={ + 'c_id': c1_table.c.id, + 'b_value': b_table.c.value, + # note we need to make this fixed with lazy=False until + # [ticket:3348] is resolved + 'c1s': relationship(C1, lazy=False, innerjoin=True) + }) + + a_mapper = inspect(A) + a_mapper.add_property( + "bs_np", relationship(b_np) + ) + + s = Session() + + q = s.query(A).options( + joinedload('bs_np', innerjoin=False) + ) + self.assert_compile( + q, + "SELECT a.id AS a_id, c1_1.id AS c1_1_id, c1_1.b_id AS c1_1_b_id, " + "c1_1.value AS c1_1_value, c1_2.id AS c1_2_id, " + "b_1.value AS b_1_value, b_1.id AS b_1_id, " + "b_1.a_id AS b_1_a_id, c1_2.b_id AS c1_2_b_id, " + "c1_2.value AS c1_2_value " + "FROM a LEFT OUTER JOIN " + "(b AS b_1 LEFT OUTER JOIN c1 AS c1_2 ON b_1.id = c1_2.b_id " + "JOIN c1 AS c1_1 ON b_1.id = c1_1.b_id) ON a.id = b_1.a_id" + ) + class SubqueryAliasingTest(fixtures.MappedTest, testing.AssertsCompiledSQL): -- 2.47.3