Commit 89bf7a45 authored by Anssi Kääriäinen's avatar Anssi Kääriäinen
Browse files

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.
parent b7bd7087

django/db/models/sql/query.py

+3 −1
Original line number Diff line number Diff line
@@ -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))

tests/queries/tests.py

+32 −17
Original line number Diff line number Diff line
@@ -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)