Fixed #21366 -- regression in join promotion logic

The regression was caused by ecaba3602837d1e02fe1e961f7d3bf9086453259
and affected OR connected filters.

2 files changed, 27 insertions, 4 deletions

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