From: Mike Bayer Date: Wed, 24 Apr 2013 22:58:09 +0000 (-0400) Subject: - attempt to replace the whole idea of "join_to_left" with a more X-Git-Tag: rel_0_8_1~3^2~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=637232709770de034caf67c9ece6121c83c43681;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - attempt to replace the whole idea of "join_to_left" with a more fundamental and general purpose heuristic. this initial approach has about 60 tests failing but seems to have gone pretty far --- diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index c80a430af1..b7441fff3b 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -1921,38 +1921,15 @@ class Query(object): info.selectable,\ getattr(info, 'is_aliased_class', False) - # 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, 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 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 - try: clause = orm_join(clause, right, - onclause, isouter=outerjoin, - join_to_left=join_to_left) + onclause, isouter=outerjoin) except sa_exc.ArgumentError, ae: raise sa_exc.InvalidRequestError( "Could not find a FROM clause to join from. " @@ -1981,8 +1958,7 @@ class Query(object): "Could not find a FROM clause to join from") try: - clause = orm_join(clause, right, onclause, - isouter=outerjoin, join_to_left=join_to_left) + clause = orm_join(clause, right, onclause, isouter=outerjoin) except sa_exc.ArgumentError, ae: raise sa_exc.InvalidRequestError( "Could not find a FROM clause to join from. " diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index f3b8e271d9..dd24d8bf44 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -872,20 +872,16 @@ class _ORMJoin(expression.Join): def __init__(self, left, right, onclause=None, isouter=False, join_to_left=True): - adapt_from = None + adapt_from = None if hasattr(left, '_orm_mappers'): left_mapper = left._orm_mappers[1] - if join_to_left: - adapt_from = left.right else: info = inspection.inspect(left) left_mapper = getattr(info, 'mapper', None) - left = info.selectable - left_is_aliased = getattr(info, 'is_aliased_class', False) - if join_to_left and (left_is_aliased or not left_mapper): - adapt_from = left + left_info = inspection.inspect(left) + left_selectable = left_info.selectable info = inspection.inspect(right) right_mapper = getattr(info, 'mapper', None) @@ -902,18 +898,34 @@ class _ORMJoin(expression.Join): if isinstance(onclause, basestring): prop = left_mapper.get_property(onclause) + on_selectable = prop.parent.selectable elif isinstance(onclause, attributes.QueryableAttribute): - if adapt_from is None: - adapt_from = onclause.comparator._source_selectable() + on_selectable = onclause.comparator._source_selectable() + #if adapt_from is None: + # adapt_from = onclause.comparator._source_selectable() prop = onclause.property elif isinstance(onclause, MapperProperty): prop = onclause + on_selectable = prop.parent.selectable else: prop = None if prop: + import pdb + pdb.set_trace() + _derived = [] + for s in expression._from_objects(left_selectable): + if s == on_selectable: + adapt_from = s + break + elif s.is_derived_from(on_selectable): + _derived.append(s) + else: + if _derived: + adapt_from = _derived[0] + pj, sj, source, dest, \ - secondary, target_adapter = prop._create_joins( + secondary, target_adapter = prop._create_joins( source_selectable=adapt_from, dest_selectable=adapt_to, source_polymorphic=True, diff --git a/test/orm/test_joins.py b/test/orm/test_joins.py index 0cb1278edd..629c55ce51 100644 --- a/test/orm/test_joins.py +++ b/test/orm/test_joins.py @@ -2057,8 +2057,8 @@ class SelfReferentialTest(fixtures.MappedTest, AssertsCompiledSQL): sess.query(parent, grandparent, Node).\ join(parent, Node.parent).\ join(grandparent, parent.parent).\ - filter(Node.data=='n122').filter(parent.data=='n12').\ - filter(grandparent.data=='n1').from_self().first(), + filter(Node.data == 'n122').filter(parent.data == 'n12').\ + filter(grandparent.data == 'n1').from_self().first(), (Node(data='n12'), Node(data='n1'), Node(data='n122')) )