Fixed #20600 -- ordered distinct(*fields) in subqueries

django/db/models/sql/compiler.py

@@ -164,7 +164,7 @@ class SQLCompiler(object):
         Used when nesting this query inside another.
         """
         obj = self.query.clone()
-        if obj.low_mark == 0 and obj.high_mark is None:
+        if obj.low_mark == 0 and obj.high_mark is None and not self.query.distinct_fields:
             # If there is no slicing in use, then we can safely drop all ordering
             obj.clear_ordering(True)
         return obj.get_compiler(connection=self.connection).as_sql()
@@ -364,6 +364,10 @@ class SQLCompiler(object):
 
         params = []
         ordering_params = []
+        # For plain DISTINCT queries any ORDER BY clause must appear
+        # in SELECT clause.
+        # http://www.postgresql.org/message-id/27009.1171559417@sss.pgh.pa.us
+        must_append_to_select = distinct and not self.query.distinct_fields
         for pos, field in enumerate(ordering):
             if field == '?':
                 result.append(self.connection.ops.random_function_sql())
@@ -388,7 +392,7 @@ class SQLCompiler(object):
                 if (table, col) not in processed_pairs:
                     elt = '%s.%s' % (qn(table), col)
                     processed_pairs.add((table, col))
-                    if not distinct or elt in select_aliases:
+                    if not must_append_to_select or elt in select_aliases:
                         result.append('%s %s' % (elt, order))
                         group_by.append((elt, []))
             elif not self.query._extra or get_order_dir(field)[0] not in self.query._extra:
@@ -400,21 +404,23 @@ class SQLCompiler(object):
                         if (table, col) not in processed_pairs:
                             elt = '%s.%s' % (qn(table), qn2(col))
                             processed_pairs.add((table, col))
-                            if distinct and elt not in select_aliases:
+                            if must_append_to_select and elt not in select_aliases:
                                 ordering_aliases.append(elt)
                             result.append('%s %s' % (elt, order))
                             group_by.append((elt, []))
             else:
                 elt = qn2(col)
                 if col not in self.query.extra_select:
-                    sql = "(%s) AS %s" % (self.query.extra[col][0], elt)
+                    if must_append_to_select:
-                    ordering_aliases.append(sql)
+                        sql = "(%s) AS %s" % (self.query.extra[col][0], elt)
-                    ordering_params.extend(self.query.extra[col][1])
+                        ordering_aliases.append(sql)
+                        ordering_params.extend(self.query.extra[col][1])
+                        result.append('%s %s' % (elt, order))
+                    else:
+                        result.append("(%s) %s" % (self.query.extra[col][0], order))
+                        params.extend(self.query.extra[col][1])
                 else:
-                    if distinct and col not in select_aliases:
+                    result.append('%s %s' % (elt, order))
-                        ordering_aliases.append(elt)
-                        ordering_params.extend(params)
-                result.append('%s %s' % (elt, order))
                 group_by.append(self.query.extra[col])
         self.ordering_aliases = ordering_aliases
         self.ordering_params = ordering_params

tests/distinct_on_fields/tests.py

@@ -16,13 +16,13 @@ class DistinctOnTests(TestCase):
         Tag.objects.create(name='t4', parent=t3)
         Tag.objects.create(name='t5', parent=t3)
 
-        p1_o1 = Staff.objects.create(id=1, name="p1", organisation="o1")
+        self.p1_o1 = Staff.objects.create(id=1, name="p1", organisation="o1")
-        p2_o1 = Staff.objects.create(id=2, name="p2", organisation="o1")
+        self.p2_o1 = Staff.objects.create(id=2, name="p2", organisation="o1")
-        p3_o1 = Staff.objects.create(id=3, name="p3", organisation="o1")
+        self.p3_o1 = Staff.objects.create(id=3, name="p3", organisation="o1")
-        Staff.objects.create(id=4, name="p1", organisation="o2")
+        self.p1_o2 = Staff.objects.create(id=4, name="p1", organisation="o2")
-        p1_o1.coworkers.add(p2_o1, p3_o1)
+        self.p1_o1.coworkers.add(self.p2_o1, self.p3_o1)
-        StaffTag.objects.create(staff=p1_o1, tag=t1)
+        StaffTag.objects.create(staff=self.p1_o1, tag=t1)
-        StaffTag.objects.create(staff=p1_o1, tag=t1)
+        StaffTag.objects.create(staff=self.p1_o1, tag=t1)
 
         celeb1 = Celebrity.objects.create(name="c1")
         celeb2 = Celebrity.objects.create(name="c2")
@@ -114,3 +114,17 @@ class DistinctOnTests(TestCase):
         # distinct + aggregate not allowed
         with self.assertRaises(NotImplementedError):
             Celebrity.objects.distinct('id').aggregate(Max('id'))
+
+    def test_distinct_on_in_ordered_subquery(self):
+        qs = Staff.objects.distinct('name').order_by('name', 'id')
+        qs = Staff.objects.filter(pk__in=qs).order_by('name')
+        self.assertQuerysetEqual(
+            qs, [self.p1_o1, self.p2_o1, self.p3_o1],
+            lambda x: x
+        )
+        qs = Staff.objects.distinct('name').order_by('name', '-id')
+        qs = Staff.objects.filter(pk__in=qs).order_by('name')
+        self.assertQuerysetEqual(
+            qs, [self.p1_o2, self.p2_o1, self.p3_o1],
+            lambda x: x
+        )

tests/extra_regress/tests.py

@@ -347,3 +347,19 @@ class ExtraRegressTests(TestCase):
             ['<TestObject: TestObject: a,a,a>', '<TestObject: TestObject: b,a,a>'],
             ordered=False
         )
+
+    def test_extra_values_distinct_ordering(self):
+        t1 = TestObject.objects.create(first='a', second='a', third='a')
+        t2 = TestObject.objects.create(first='a', second='b', third='b')
+        qs = TestObject.objects.extra(
+            select={'second_extra': 'second'}
+        ).values_list('id', flat=True).distinct()
+        self.assertQuerysetEqual(
+            qs.order_by('second_extra'), [t1.pk, t2.pk], lambda x: x)
+        self.assertQuerysetEqual(
+            qs.order_by('-second_extra'), [t2.pk, t1.pk], lambda x: x)
+        # Note: the extra ordering must appear in select clause, so we get two
+        # non-distinct results here (this is on purpose, see #7070).
+        self.assertQuerysetEqual(
+            qs.order_by('-second_extra').values_list('first', flat=True),
+            ['a', 'a'], lambda x: x)