Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #8790 -- Multi-branch join trees that shared tables of the same…

… name were

sometimes also sharing aliases, instead of creating their own. This was
generating incorrect SQL.

No representative test for this fix yet because I haven't had time to write one
that fits in nicely with the test suite. But it works for the monstrous example
in #8790 and a bunch of other complex examples I've created locally. Will write
a test later.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@8853 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 5c32fe7fad5c52fb5b62dc97503aed516bb9c668 1 parent 2a14acb
Malcolm Tredinnick malcolmt authored
Showing with 46 additions and 39 deletions.
  1. +46 −39 django/db/models/sql/query.py
85 django/db/models/sql/query.py
View
@@ -658,10 +658,7 @@ def find_ordering_name(self, name, opts, alias=None, default_order='ASC',
self.ref_alias(alias)
# Must use left outer joins for nullable fields.
- must_promote = False
- for join in joins:
- if self.promote_alias(join, must_promote):
- must_promote = True
+ self.promote_alias_chain(joins)
# If we get to this point and the field is a relation to another model,
# append the default ordering for that model.
@@ -744,6 +741,38 @@ def promote_alias(self, alias, unconditional=False):
return True
return False
+ def promote_alias_chain(self, chain, must_promote=False):
+ """
+ Walks along a chain of aliases, promoting the first nullable join and
+ any joins following that. If 'must_promote' is True, all the aliases in
+ the chain are promoted.
+ """
+ for alias in chain:
+ if self.promote_alias(alias, must_promote):
+ must_promote = True
+
+ def promote_unused_aliases(self, initial_refcounts, used_aliases):
+ """
+ Given a "before" copy of the alias_refcounts dictionary (as
+ 'initial_refcounts') and a collection of aliases that may have been
+ changed or created, works out which aliases have been created since
+ then and which ones haven't been used and promotes all of those
+ aliases, plus any children of theirs in the alias tree, to outer joins.
+ """
+ # FIXME: There's some (a lot of!) overlap with the similar OR promotion
+ # in add_filter(). It's not quite identical, but is very similar. So
+ # pulling out the common bits is something for later.
+ considered = {}
+ for alias in self.tables:
+ if alias not in used_aliases:
+ continue
+ if (alias not in initial_refcounts or
+ self.alias_refcount[alias] == initial_refcounts[alias]):
+ parent = self.alias_map[alias][LHS_ALIAS]
+ must_promote = considered.get(parent, False)
+ promoted = self.promote_alias(alias, must_promote)
+ considered[alias] = must_promote or promoted
+
def change_aliases(self, change_map):
"""
Changes the aliases in change_map (which maps old-alias -> new-alias),
@@ -807,10 +836,11 @@ def bump_prefix(self, exceptions=()):
The 'exceptions' parameter is a container that holds alias names which
should not be changed.
"""
- assert ord(self.alias_prefix) < ord('Z')
- self.alias_prefix = chr(ord(self.alias_prefix) + 1)
+ current = ord(self.alias_prefix)
+ assert current < ord('Z')
+ prefix = chr(current + 1)
+ self.alias_prefix = prefix
change_map = {}
- prefix = self.alias_prefix
for pos, alias in enumerate(self.tables):
if alias in exceptions:
continue
@@ -888,6 +918,8 @@ def join(self, connection, always_create=False, exclusions=(),
# The LHS of this join tuple is no longer part of the
# query, so skip this possibility.
continue
+ if self.alias_map[alias][LHS_ALIAS] != lhs:
+ continue
self.ref_alias(alias)
if promote:
self.promote_alias(alias)
@@ -1120,6 +1152,7 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
table_it = iter(self.tables)
join_it.next(), table_it.next()
table_promote = False
+ join_promote = False
for join in join_it:
table = table_it.next()
if join == table and self.alias_refcount[join] > 1:
@@ -1128,20 +1161,13 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
if table != join:
table_promote = self.promote_alias(table)
break
- for join in join_it:
- if self.promote_alias(join, join_promote):
- join_promote = True
- for table in table_it:
- # Some of these will have been promoted from the join_list, but
- # that's harmless.
- if self.promote_alias(table, table_promote):
- table_promote = True
+ self.promote_alias_chain(join_it, join_promote)
+ self.promote_alias_chain(table_it, table_promote)
self.where.add((alias, col, field, lookup_type, value), connector)
if negate:
- for alias in join_list:
- self.promote_alias(alias)
+ self.promote_alias_chain(join_list)
if lookup_type != 'isnull':
if final > 1:
for alias in join_list:
@@ -1201,22 +1227,7 @@ def add_q(self, q_object, used_aliases=None):
# be promoted to outer joins if they are nullable relations.
# (they shouldn't turn the whole conditional into the empty
# set just because they don't match anything).
- # FIXME: There's some (a lot of!) overlap with the similar
- # OR promotion in add_filter(). It's not quite identical,
- # but is very similar. So pulling out the common bits is
- # something for later (code smell: too much indentation
- # here)
- considered = {}
- for alias in self.tables:
- if alias not in used_aliases:
- continue
- if (alias not in refcounts_before or
- self.alias_refcount[alias] ==
- refcounts_before[alias]):
- parent = self.alias_map[alias][LHS_ALIAS]
- must_promote = considered.get(parent, False)
- promoted = self.promote_alias(alias, must_promote)
- considered[alias] = must_promote or promoted
+ self.promote_unused_aliases(refcounts_before, used_aliases)
connector = q_object.connector
if q_object.negated:
self.where.negate()
@@ -1439,6 +1450,7 @@ def split_exclude(self, filter_expr, prefix, can_reuse):
"""
query = Query(self.model, self.connection)
query.add_filter(filter_expr, can_reuse=can_reuse)
+ query.bump_prefix()
query.set_start(prefix)
query.clear_ordering(True)
self.add_filter(('%s__in' % prefix, query), negate=True, trim=True,
@@ -1500,12 +1512,7 @@ def add_fields(self, field_names, allow_m2m=True):
final_alias = join[LHS_ALIAS]
col = join[LHS_JOIN_COL]
joins = joins[:-1]
- promote = False
- for join in joins[1:]:
- # Only nullable aliases are promoted, so we don't end up
- # doing unnecessary left outer joins here.
- if self.promote_alias(join, promote):
- promote = True
+ self.promote_alias_chain(joins[1:])
self.select.append((final_alias, col))
self.select_fields.append(field)
except MultiJoin:
Please sign in to comment.
Something went wrong with that request. Please try again.