diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 37f99c735c..493856fb41 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -740,7 +740,7 @@ class Query(object): self.unref_alias(alias, unref_amount) def promote_disjunction(self, aliases_before, alias_usage_counts, - num_childs): + num_childs, left_joins_before): """ This method is to be used for promoting joins in ORed filters. @@ -749,7 +749,8 @@ class Query(object): and isn't pre-existing needs to be promoted to LOUTER join. """ for alias, use_count in alias_usage_counts.items(): - if use_count < num_childs and alias not in aliases_before: + if ((use_count < num_childs and alias not in aliases_before) + or alias in left_joins_before): self.promote_joins([alias]) def change_aliases(self, change_map): @@ -1314,9 +1315,14 @@ class Query(object): if connector == OR: alias_usage_counts = dict() aliases_before = set(self.tables) + left_joins_before = set() for child in q_object.children: if connector == OR: refcounts_before = self.alias_refcount.copy() + left_joins_before = left_joins_before.union(set( + t for t in self.alias_map + if self.alias_map[t].join_type == self.LOUTER and + self.alias_refcount[t] > 0)) if isinstance(child, Node): child_clause = self._add_q( child, used_aliases, branch_negated, @@ -1332,7 +1338,7 @@ class Query(object): alias_usage_counts[alias] = alias_usage_counts.get(alias, 0) + 1 if connector == OR: self.promote_disjunction(aliases_before, alias_usage_counts, - len(q_object.children)) + len(q_object.children), left_joins_before) return target_clause def names_to_path(self, names, opts, allow_many): diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 090fedb430..74c73297ed 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2719,6 +2719,23 @@ class NullJoinPromotionOrTest(TestCase): qs = ModelA.objects.filter(Q(b__name__isnull=True) | Q(b__name__isnull=False)) self.assertTrue(' LEFT OUTER JOIN ' in str(qs.query)) + def test_ticket_21366(self): + n = Note.objects.create(note='n', misc='m') + e = ExtraInfo.objects.create(info='info', note=n) + a = Author.objects.create(name='Author1', num=1, extra=e) + Ranking.objects.create(rank=1, author=a) + r1 = Report.objects.create(name='Foo', creator=a) + r2 = Report.objects.create(name='Bar') + Report.objects.create(name='Bar', creator=a) + qs = Report.objects.filter( + Q(creator__ranking__isnull=True) | + Q(creator__ranking__rank=1, name='Foo') + ) + self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 2) + self.assertEqual(str(qs.query).count(' JOIN '), 2) + self.assertQuerysetEqual( + qs.order_by('name'), [r2, r1], lambda x: x) + class ReverseJoinTrimmingTest(TestCase): def test_reverse_trimming(self): # Check that we don't accidentally trim reverse joins - we can't know @@ -2837,7 +2854,6 @@ class DisjunctionPromotionTests(TestCase): self.assertEqual(str(qs.query).count('INNER JOIN'), 1) def test_disjunction_promotion5_demote(self): - # Failure because no join demotion logic for this case. qs = BaseA.objects.filter(Q(a=1) | Q(a=2)) # Note that the above filters on a force the join to an # inner join even if it is trimmed. @@ -2845,6 +2861,7 @@ class DisjunctionPromotionTests(TestCase): qs = qs.filter(Q(a__f1='foo') | Q(b__f1='foo')) # So, now the a__f1 join doesn't need promotion. self.assertEqual(str(qs.query).count('INNER JOIN'), 1) + # But b__f1 does. self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 1) qs = BaseA.objects.filter(Q(a__f1='foo') | Q(b__f1='foo')) # Now the join to a is created as LOUTER