diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 36d45b7cb61..5801b2f4280 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -455,6 +455,9 @@ class SQLCompiler(object): alias = self.query.get_initial_alias() field, target, opts, joins, _, _ = self.query.setup_joins(pieces, opts, alias, False) + # We will later on need to promote those joins that were added to the + # query afresh above. + joins_to_promote = [j for j in joins if self.query.alias_refcount[j] < 2] alias = joins[-1] col = target.column if not field.rel: @@ -466,8 +469,9 @@ class SQLCompiler(object): # Must use left outer joins for nullable fields and their relations. # Ordering or distinct must not affect the returned set, and INNER # JOINS for nullable fields could do this. - self.query.promote_alias_chain(joins, - self.query.alias_map[joins[0]].join_type == self.query.LOUTER) + if joins_to_promote: + self.query.promote_alias_chain(joins_to_promote, + self.query.alias_map[joins_to_promote[0]].join_type == self.query.LOUTER) return field, col, alias, joins, opts def _final_join_removal(self, col, alias): diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py index 07aa6db67b0..e4ce9d7b60c 100644 --- a/tests/regressiontests/queries/models.py +++ b/tests/regressiontests/queries/models.py @@ -264,7 +264,7 @@ class SingleObject(models.Model): return self.name class RelatedObject(models.Model): - single = models.ForeignKey(SingleObject) + single = models.ForeignKey(SingleObject, null=True) class Meta: ordering = ['single'] diff --git a/tests/regressiontests/queries/tests.py b/tests/regressiontests/queries/tests.py index 14a693f2c5f..28aa63328ea 100644 --- a/tests/regressiontests/queries/tests.py +++ b/tests/regressiontests/queries/tests.py @@ -19,7 +19,8 @@ from .models import (Annotation, Article, Author, Celebrity, Child, Cover, 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) + SpecialCategory, OneToOneCategory, NullableName, ProxyCategory, + SingleObject, RelatedObject) class BaseQuerysetTest(TestCase): @@ -1321,12 +1322,33 @@ class NullableRelOrderingTests(TestCase): def test_ticket10028(self): # Ordering by model related to nullable relations(!) should use outer # joins, so that all results are included. - _ = Plaything.objects.create(name="p1") + Plaything.objects.create(name="p1") self.assertQuerysetEqual( Plaything.objects.all(), [''] ) + def test_join_already_in_query(self): + # Ordering by model related to nullable relations should not change + # the join type of already existing joins. + Plaything.objects.create(name="p1") + s = SingleObject.objects.create(name='s') + r = RelatedObject.objects.create(single=s) + Plaything.objects.create(name="p2", others=r) + qs = Plaything.objects.all().filter(others__isnull=False).order_by('pk') + self.assertTrue('INNER' in str(qs.query)) + qs = qs.order_by('others__single__name') + # The ordering by others__single__pk will add one new join (to single) + # and that join must be LEFT join. The already existing join to related + # objects must be kept INNER. So, we have both a INNER and a LEFT join + # in the query. + self.assertTrue('LEFT' in str(qs.query)) + self.assertTrue('INNER' in str(qs.query)) + self.assertQuerysetEqual( + qs, + [''] + ) + class DisjunctiveFilterTests(TestCase): def setUp(self):