From 89bf7a45250cf554295f3d17d708311d3edba101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Mon, 10 Jun 2013 18:22:30 +0300 Subject: [PATCH] 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. --- django/db/models/sql/query.py | 4 ++- tests/queries/tests.py | 49 +++++++++++++++++++++++------------ 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)