Fixed #20528 -- regression in select_related join promotion

The join used by select_related was incorrectly INNER when the query
had an ORed filter for nullable join that was trimmed away. Fixed this
by forcing the join type to LOUTER even when a join was trimmed away
in ORed queries.

2 files changed, 35 insertions, 18 deletions

diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index 154b6bd204..d26b8d5c95 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -1913,5 +1913,7 @@ def alias_diff(refcounts_before, refcounts_after):
     Given the before and after copies of refcounts works out which aliases
     have been added to the after copy.
     """
+    # Use -1 as default value so that any join that is created, then trimmed
+    # is seen as added.
     return set(t for t in refcounts_after
-               if refcounts_after[t] > refcounts_before.get(t, 0))
+               if refcounts_after[t] > refcounts_before.get(t, -1))
diff --git a/tests/queries/tests.py b/tests/queries/tests.py
index 481b690c20..612c268925 100644
--- a/tests/queries/tests.py
+++ b/tests/queries/tests.py
@@ -16,15 +16,16 @@ from django.test.utils import str_prefix
 from django.utils import unittest
 from django.utils.datastructures import SortedDict
 
-from .models import (Annotation, Article, Author, Celebrity, Child, Cover,
-    Detail, DumbCategory, ExtraInfo, Fan, Item, LeafA, Join, LeafB, LoopX, LoopZ,
-    ManagedModel, Member, NamedCategory, Note, Number, Plaything, PointerA,
-    Ranking, Related, Report, ReservedName, Tag, TvChef, Valid, X, Food, Eaten,
-    Node, ObjectA, ObjectB, ObjectC, CategoryItem, SimpleCategory,
-    SpecialCategory, OneToOneCategory, NullableName, ProxyCategory,
-    SingleObject, RelatedObject, ModelA, ModelB, ModelC, ModelD, Responsibility,
-    Job, JobResponsibilities, BaseA, Identifier, Program, Channel, Page,
-    Paragraph, Chapter, Book, MyObject, Order, OrderItem)
+from .models import (
+    Annotation, Article, Author, Celebrity, Child, Cover, Detail, DumbCategory,
+    ExtraInfo, Fan, Item, LeafA, Join, LeafB, LoopX, LoopZ, ManagedModel,
+    Member, NamedCategory, Note, Number, Plaything, PointerA, Ranking, Related,
+    Report, ReservedName, Tag, TvChef, Valid, X, Food, Eaten, Node, ObjectA,
+    ObjectB, ObjectC, CategoryItem, SimpleCategory, SpecialCategory,
+    OneToOneCategory, NullableName, ProxyCategory, SingleObject, RelatedObject,
+    ModelA, ModelB, ModelC, ModelD, Responsibility, Job, JobResponsibilities,
+    BaseA, FK1, Identifier, Program, Channel, Page, Paragraph, Chapter, Book,
+    MyObject, Order, OrderItem)
 
 
 class BaseQuerysetTest(TestCase):
@@ -2620,6 +2621,19 @@ class JoinReuseTest(TestCase):
         self.assertEqual(str(qs.query).count('JOIN'), 2)
 
 class DisjunctionPromotionTests(TestCase):
+    def test_disjuction_promotion_select_related(self):
+        fk1 = FK1.objects.create(f1='f1', f2='f2')
+        basea = BaseA.objects.create(a=fk1)
+        qs = BaseA.objects.filter(Q(a=fk1) | Q(b=2))
+        self.assertEqual(str(qs.query).count(' JOIN '), 0)
+        qs = qs.select_related('a', 'b')
+        self.assertEqual(str(qs.query).count(' INNER JOIN '), 0)
+        self.assertEqual(str(qs.query).count(' LEFT OUTER JOIN '), 2)
+        with self.assertNumQueries(1):
+            self.assertQuerysetEqual(qs, [basea], lambda x: x)
+            self.assertEqual(qs[0].a, fk1)
+            self.assertIs(qs[0].b, None)
+
     def test_disjunction_promotion1(self):
         # Pre-existing join, add two ORed filters to the same join,
         # all joins can be INNER JOINS.
@@ -2669,17 +2683,23 @@ class DisjunctionPromotionTests(TestCase):
         self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
         self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 1)
 
-    def test_disjunction_promotion4(self):
+    @unittest.expectedFailure
+    def test_disjunction_promotion4_failing(self):
+        # Failure because no join repromotion
         qs = BaseA.objects.filter(Q(a=1) | Q(a=2))
         self.assertEqual(str(qs.query).count('JOIN'), 0)
         qs = qs.filter(a__f1='foo')
         self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
+
+    def test_disjunction_promotion4(self):
         qs = BaseA.objects.filter(a__f1='foo')
         self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
         qs = qs.filter(Q(a=1) | Q(a=2))
         self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
 
-    def test_disjunction_promotion5(self):
+    @unittest.expectedFailure
+    def test_disjunction_promotion5_failing(self):
+        # Failure because no join repromotion logic.
         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.
@@ -2688,15 +2708,10 @@ class DisjunctionPromotionTests(TestCase):
         # So, now the a__f1 join doesn't need promotion.
         self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
         self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 1)
-
-    @unittest.expectedFailure
-    def test_disjunction_promotion5_failing(self):
         qs = BaseA.objects.filter(Q(a__f1='foo') | Q(b__f1='foo'))
         # Now the join to a is created as LOUTER
         self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 0)
-        # The below filter should force the a to be inner joined. But,
-        # this is failing as we do not have join unpromotion logic.
-        qs = BaseA.objects.filter(Q(a=1) | Q(a=2))
+        qs = qs.objects.filter(Q(a=1) | Q(a=2))
         self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
         self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 1)