Skip to content

Commit

Permalink
Removed join() promote kwarg
Browse files Browse the repository at this point in the history
The join promotion was over-aggressive in select_related handling, and
the sole user of promote was comine(), but it is easy enough to handle
the force-promotion in there manually.
  • Loading branch information
akaariai committed Feb 19, 2013
1 parent 4d8ed81 commit f7f7044
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 18 deletions.
4 changes: 2 additions & 2 deletions django/db/models/sql/compiler.py
Expand Up @@ -620,7 +620,7 @@ def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,


alias = self.query.join((alias, table, f.column, alias = self.query.join((alias, table, f.column,
f.rel.get_related_field().column), f.rel.get_related_field().column),
promote=promote, join_field=f) outer_if_first=promote, join_field=f)
columns, aliases = self.get_default_columns(start_alias=alias, columns, aliases = self.get_default_columns(start_alias=alias,
opts=f.rel.to._meta, as_pairs=True) opts=f.rel.to._meta, as_pairs=True)
self.query.related_select_cols.extend( self.query.related_select_cols.extend(
Expand Down Expand Up @@ -648,7 +648,7 @@ def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,
table = model._meta.db_table table = model._meta.db_table
alias = self.query.join( alias = self.query.join(
(alias, table, f.rel.get_related_field().column, f.column), (alias, table, f.rel.get_related_field().column, f.column),
promote=True, join_field=f outer_if_first=True, join_field=f
) )
from_parent = (opts.model if issubclass(model, opts.model) from_parent = (opts.model if issubclass(model, opts.model)
else None) else None)
Expand Down
25 changes: 9 additions & 16 deletions django/db/models/sql/query.py
Expand Up @@ -507,9 +507,11 @@ def combine(self, rhs, connector):
# updated alias. # updated alias.
lhs = change_map.get(lhs, lhs) lhs = change_map.get(lhs, lhs)
new_alias = self.join( new_alias = self.join(
(lhs, table, lhs_col, col), reuse=reuse, promote=promote, (lhs, table, lhs_col, col), reuse=reuse,
outer_if_first=not conjunction, nullable=nullable, outer_if_first=not conjunction, nullable=nullable,
join_field=join_field) join_field=join_field)
if promote:
self.promote_joins([new_alias])
# We can't reuse the same join again in the query. If we have two # We can't reuse the same join again in the query. If we have two
# distinct joins for the same connection in rhs query, then the # distinct joins for the same connection in rhs query, then the
# combined query must have two joins, too. # combined query must have two joins, too.
Expand Down Expand Up @@ -914,8 +916,8 @@ def count_active_tables(self):
""" """
return len([1 for count in self.alias_refcount.values() if count]) return len([1 for count in self.alias_refcount.values() if count])


def join(self, connection, reuse=None, promote=False, def join(self, connection, reuse=None, outer_if_first=False,
outer_if_first=False, nullable=False, join_field=None): nullable=False, join_field=None):
""" """
Returns an alias for the join in 'connection', either reusing an Returns an alias for the join in 'connection', either reusing an
existing alias for that join or creating a new one. 'connection' is a existing alias for that join or creating a new one. 'connection' is a
Expand All @@ -929,14 +931,8 @@ def join(self, connection, reuse=None, promote=False,
(matching the connection) are reusable, or it can be a set containing (matching the connection) are reusable, or it can be a set containing
the aliases that can be reused. the aliases that can be reused.
If 'promote' is True, the join type for the alias will be LOUTER (if
the alias previously existed, the join type will be promoted from INNER
to LOUTER, if necessary).
If 'outer_if_first' is True and a new join is created, it will have the If 'outer_if_first' is True and a new join is created, it will have the
LOUTER join type. Used for example when adding ORed filters, where we LOUTER join type.
want to use LOUTER joins except if some other join already restricts
the join to INNER join.
A join is always created as LOUTER if the lhs alias is LOUTER to make A join is always created as LOUTER if the lhs alias is LOUTER to make
sure we do not generate chains like t1 LOUTER t2 INNER t3. sure we do not generate chains like t1 LOUTER t2 INNER t3.
Expand All @@ -961,8 +957,6 @@ def join(self, connection, reuse=None, promote=False,
# join_field used for the under work join. # join_field used for the under work join.
continue continue
self.ref_alias(alias) self.ref_alias(alias)
if promote or (lhs and self.alias_map[lhs].join_type == self.LOUTER):
self.promote_joins([alias])
return alias return alias


# No reuse is possible, so we need a new alias. # No reuse is possible, so we need a new alias.
Expand All @@ -971,10 +965,9 @@ def join(self, connection, reuse=None, promote=False,
# Not all tables need to be joined to anything. No join type # Not all tables need to be joined to anything. No join type
# means the later columns are ignored. # means the later columns are ignored.
join_type = None join_type = None
elif (promote or outer_if_first elif outer_if_first or self.alias_map[lhs].join_type == self.LOUTER:
or self.alias_map[lhs].join_type == self.LOUTER): # We need to use LOUTER join if asked by outer_if_first or if the
# We need to use LOUTER join if asked by promote or outer_if_first, # LHS table is left-joined in the query.
# or if the LHS table is left-joined in the query.
join_type = self.LOUTER join_type = self.LOUTER
else: else:
join_type = self.INNER join_type = self.INNER
Expand Down
23 changes: 23 additions & 0 deletions tests/regressiontests/select_related_regress/tests.py
Expand Up @@ -139,3 +139,26 @@ def test_regression_12851(self):
self.assertEqual(troy.name, 'Troy Buswell') self.assertEqual(troy.name, 'Troy Buswell')
self.assertEqual(troy.value, 42) self.assertEqual(troy.value, 42)
self.assertEqual(troy.state.name, 'Western Australia') self.assertEqual(troy.state.name, 'Western Australia')

def test_null_join_promotion(self):
australia = Country.objects.create(name='Australia')
active = ClientStatus.objects.create(name='active')

wa = State.objects.create(name="Western Australia", country=australia)
bob = Client.objects.create(name='Bob', status=active)
jack = Client.objects.create(name='Jack', status=active, state=wa)
qs = Client.objects.filter(state=wa).select_related('state')
with self.assertNumQueries(1):
self.assertEqual(list(qs), [jack])
self.assertEqual(qs[0].state, wa)
# The select_related join wasn't promoted as there was already an
# existing (even if trimmed) inner join to state.
self.assertFalse('LEFT OUTER' in str(qs.query))
qs = Client.objects.select_related('state').order_by('name')
with self.assertNumQueries(1):
self.assertEqual(list(qs), [bob, jack])
self.assertIs(qs[0].state, None)
self.assertEqual(qs[1].state, wa)
# The select_related join was promoted as there is already an
# existing join.
self.assertTrue('LEFT OUTER' in str(qs.query))

0 comments on commit f7f7044

Please sign in to comment.