From a193372753ad9d1d15ad5e2d6d06bbc07ca3f433 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Tue, 21 Aug 2012 18:43:08 +0300 Subject: [PATCH] Fixed #17886 -- Fixed join promotion in ORed nullable queries The ORM generated a query with INNER JOIN instead of LEFT OUTER JOIN in a somewhat complicated case. The main issue was that there was a chain of nullable FK -> non-nullble FK, and the join promotion logic didn't see the need to promote the non-nullable FK even if the previous nullable FK was already promoted to LOUTER JOIN. This resulted in a query like a LOUTER b INNER c, which incorrectly prunes results. --- django/db/models/sql/query.py | 7 ++++++- tests/regressiontests/queries/models.py | 15 +++++++++++++++ tests/regressiontests/queries/tests.py | 25 ++++++++++++++++++++++++- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index c62c9ac23e..27a4ac9ce5 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -910,7 +910,12 @@ class Query(object): # 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: + 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. Adding inner join + # to an existing outer join effectively cancels the effect of the + # outer join. join_type = self.LOUTER else: join_type = self.INNER diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py index 45a48ee77c..f0178a0256 100644 --- a/tests/regressiontests/queries/models.py +++ b/tests/regressiontests/queries/models.py @@ -385,3 +385,18 @@ class NullableName(models.Model): class Meta: ordering = ['id'] + +class ModelD(models.Model): + name = models.TextField() + +class ModelC(models.Model): + name = models.TextField() + +class ModelB(models.Model): + name = models.TextField() + c = models.ForeignKey(ModelC) + +class ModelA(models.Model): + name = models.TextField() + b = models.ForeignKey(ModelB, null=True) + d = models.ForeignKey(ModelD) diff --git a/tests/regressiontests/queries/tests.py b/tests/regressiontests/queries/tests.py index 85ea4aa452..005aa9650b 100644 --- a/tests/regressiontests/queries/tests.py +++ b/tests/regressiontests/queries/tests.py @@ -23,7 +23,7 @@ from .models import (Annotation, Article, Author, Celebrity, Child, Cover, Ranking, Related, Report, ReservedName, Tag, TvChef, Valid, X, Food, Eaten, Node, ObjectA, ObjectB, ObjectC, CategoryItem, SimpleCategory, SpecialCategory, OneToOneCategory, NullableName, ProxyCategory, - SingleObject, RelatedObject) + SingleObject, RelatedObject, ModelA, ModelD) class BaseQuerysetTest(TestCase): @@ -2105,3 +2105,26 @@ class WhereNodeTest(TestCase): self.assertEqual(w.as_sql(qn, connection), (None, [])) w = WhereNode(children=[empty_w, NothingNode()], connector='OR') self.assertRaises(EmptyResultSet, w.as_sql, qn, connection) + +class NullJoinPromotionOrTest(TestCase): + def setUp(self): + d = ModelD.objects.create(name='foo') + ModelA.objects.create(name='bar', d=d) + + def test_ticket_17886(self): + # The first Q-object is generating the match, the rest of the filters + # should not remove the match even if they do not match anything. The + # problem here was that b__name generates a LOUTER JOIN, then + # b__c__name generates join to c, which the ORM tried to promote but + # failed as that join isn't nullable. + q_obj = ( + Q(d__name='foo')| + Q(b__name='foo')| + Q(b__c__name='foo') + ) + qset = ModelA.objects.filter(q_obj) + self.assertEqual(len(qset), 1) + # We generate one INNER JOIN to D. The join is direct and not nullable + # so we can use INNER JOIN for it. However, we can NOT use INNER JOIN + # for the b->c join, as a->b is nullable. + self.assertEqual(str(qset.query).count('INNER JOIN'), 1)