Fixed qs.order_by() join promotion for already existing joins

When order_by causes new joins to be added to the query, the joins must
be LEFT OUTER joins for nullable relations, otherwise the order_by
could cause the results to be altered. This commit fixes the logic to
only promote new joins, previously all joins in the order_by lookup
path were promoted.

Thanks to Bruno Desthuilliers for spotting this corner case.

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):

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']

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(),
             ['<Plaything: p1>']
         )
 
+    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,
+            ['<Plaything: p2>']
+        )
+
 
 class DisjunctiveFilterTests(TestCase):
     def setUp(self):