Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Removed join() promote kwarg

The join promote=True was over-aggressive in select_related handling.
After that was removed, the only other user was query.combine(). That
use case is very easy to handle locally, so there is no more need for
the join(promote=True) flag.

Refs #19849.
  • Loading branch information...
commit edf93127bf2f9dc35b45cdea5d39a1b417ab1638 1 parent 3fef304
@akaariai akaariai authored
View
4 django/db/models/sql/compiler.py
@@ -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,
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,
opts=f.rel.to._meta, as_pairs=True)
self.query.related_select_cols.extend(
@@ -648,7 +648,7 @@ def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,
table = model._meta.db_table
alias = self.query.join(
(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)
else None)
View
25 django/db/models/sql/query.py
@@ -507,9 +507,11 @@ def combine(self, rhs, connector):
# updated alias.
lhs = change_map.get(lhs, lhs)
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,
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
# distinct joins for the same connection in rhs query, then the
# combined query must have two joins, too.
@@ -914,8 +916,8 @@ def count_active_tables(self):
"""
return len([1 for count in self.alias_refcount.values() if count])
- def join(self, connection, reuse=None, promote=False,
- outer_if_first=False, nullable=False, join_field=None):
+ def join(self, connection, reuse=None, outer_if_first=False,
+ nullable=False, join_field=None):
"""
Returns an alias for the join in 'connection', either reusing an
existing alias for that join or creating a new one. 'connection' is a
@@ -929,14 +931,8 @@ def join(self, connection, reuse=None, promote=False,
(matching the connection) are reusable, or it can be a set containing
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
- LOUTER join type. Used for example when adding ORed filters, where we
- want to use LOUTER joins except if some other join already restricts
- the join to INNER join.
+ LOUTER join type.
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.
@@ -961,8 +957,6 @@ def join(self, connection, reuse=None, promote=False,
# join_field used for the under work join.
continue
self.ref_alias(alias)
- if promote or (lhs and self.alias_map[lhs].join_type == self.LOUTER):
- self.promote_joins([alias])
return alias
# No reuse is possible, so we need a new alias.
@@ -971,10 +965,9 @@ def join(self, connection, reuse=None, promote=False,
# Not all tables need to be joined to anything. No join type
# means the later columns are ignored.
join_type = None
- elif (promote or outer_if_first
- or self.alias_map[lhs].join_type == self.LOUTER):
- # We need to use LOUTER join if asked by promote or outer_if_first,
- # or if the LHS table is left-joined in the query.
+ elif outer_if_first or self.alias_map[lhs].join_type == self.LOUTER:
+ # We need to use LOUTER join if asked by outer_if_first or if the
+ # LHS table is left-joined in the query.
join_type = self.LOUTER
else:
join_type = self.INNER
View
23 tests/regressiontests/select_related_regress/tests.py
@@ -139,3 +139,26 @@ def test_regression_12851(self):
self.assertEqual(troy.name, 'Troy Buswell')
self.assertEqual(troy.value, 42)
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))
Please sign in to comment.
Something went wrong with that request. Please try again.