Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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

  • Loading branch information...
commit b1b04df06522b092a9b4768d2d61c52956e00eca 1 parent ccbba98
Anssi Kääriäinen authored October 11, 2013
26  django/db/models/sql/compiler.py
@@ -164,7 +164,7 @@ def as_nested_sql(self):
164 164
         Used when nesting this query inside another.
165 165
         """
166 166
         obj = self.query.clone()
167  
-        if obj.low_mark == 0 and obj.high_mark is None:
  167
+        if obj.low_mark == 0 and obj.high_mark is None and not self.query.distinct_fields:
168 168
             # If there is no slicing in use, then we can safely drop all ordering
169 169
             obj.clear_ordering(True)
170 170
         return obj.get_compiler(connection=self.connection).as_sql()
@@ -364,6 +364,10 @@ def get_ordering(self):
364 364
 
365 365
         params = []
366 366
         ordering_params = []
  367
+        # For plain DISTINCT queries any ORDER BY clause must appear
  368
+        # in SELECT clause.
  369
+        # http://www.postgresql.org/message-id/27009.1171559417@sss.pgh.pa.us
  370
+        must_append_to_select = distinct and not self.query.distinct_fields
367 371
         for pos, field in enumerate(ordering):
368 372
             if field == '?':
369 373
                 result.append(self.connection.ops.random_function_sql())
@@ -388,7 +392,7 @@ def get_ordering(self):
388 392
                 if (table, col) not in processed_pairs:
389 393
                     elt = '%s.%s' % (qn(table), col)
390 394
                     processed_pairs.add((table, col))
391  
-                    if not distinct or elt in select_aliases:
  395
+                    if not must_append_to_select or elt in select_aliases:
392 396
                         result.append('%s %s' % (elt, order))
393 397
                         group_by.append((elt, []))
394 398
             elif not self.query._extra or get_order_dir(field)[0] not in self.query._extra:
@@ -400,21 +404,23 @@ def get_ordering(self):
400 404
                         if (table, col) not in processed_pairs:
401 405
                             elt = '%s.%s' % (qn(table), qn2(col))
402 406
                             processed_pairs.add((table, col))
403  
-                            if distinct and elt not in select_aliases:
  407
+                            if must_append_to_select and elt not in select_aliases:
404 408
                                 ordering_aliases.append(elt)
405 409
                             result.append('%s %s' % (elt, order))
406 410
                             group_by.append((elt, []))
407 411
             else:
408 412
                 elt = qn2(col)
409 413
                 if col not in self.query.extra_select:
410  
-                    sql = "(%s) AS %s" % (self.query.extra[col][0], elt)
411  
-                    ordering_aliases.append(sql)
412  
-                    ordering_params.extend(self.query.extra[col][1])
  414
+                    if must_append_to_select:
  415
+                        sql = "(%s) AS %s" % (self.query.extra[col][0], elt)
  416
+                        ordering_aliases.append(sql)
  417
+                        ordering_params.extend(self.query.extra[col][1])
  418
+                        result.append('%s %s' % (elt, order))
  419
+                    else:
  420
+                        result.append("(%s) %s" % (self.query.extra[col][0], order))
  421
+                        params.extend(self.query.extra[col][1])
413 422
                 else:
414  
-                    if distinct and col not in select_aliases:
415  
-                        ordering_aliases.append(elt)
416  
-                        ordering_params.extend(params)
417  
-                result.append('%s %s' % (elt, order))
  423
+                    result.append('%s %s' % (elt, order))
418 424
                 group_by.append(self.query.extra[col])
419 425
         self.ordering_aliases = ordering_aliases
420 426
         self.ordering_params = ordering_params
28  tests/distinct_on_fields/tests.py
@@ -16,13 +16,13 @@ def setUp(self):
16 16
         Tag.objects.create(name='t4', parent=t3)
17 17
         Tag.objects.create(name='t5', parent=t3)
18 18
 
19  
-        p1_o1 = Staff.objects.create(id=1, name="p1", organisation="o1")
20  
-        p2_o1 = Staff.objects.create(id=2, name="p2", organisation="o1")
21  
-        p3_o1 = Staff.objects.create(id=3, name="p3", organisation="o1")
22  
-        Staff.objects.create(id=4, name="p1", organisation="o2")
23  
-        p1_o1.coworkers.add(p2_o1, p3_o1)
24  
-        StaffTag.objects.create(staff=p1_o1, tag=t1)
25  
-        StaffTag.objects.create(staff=p1_o1, tag=t1)
  19
+        self.p1_o1 = Staff.objects.create(id=1, name="p1", organisation="o1")
  20
+        self.p2_o1 = Staff.objects.create(id=2, name="p2", organisation="o1")
  21
+        self.p3_o1 = Staff.objects.create(id=3, name="p3", organisation="o1")
  22
+        self.p1_o2 = Staff.objects.create(id=4, name="p1", organisation="o2")
  23
+        self.p1_o1.coworkers.add(self.p2_o1, self.p3_o1)
  24
+        StaffTag.objects.create(staff=self.p1_o1, tag=t1)
  25
+        StaffTag.objects.create(staff=self.p1_o1, tag=t1)
26 26
 
27 27
         celeb1 = Celebrity.objects.create(name="c1")
28 28
         celeb2 = Celebrity.objects.create(name="c2")
@@ -114,3 +114,17 @@ def test_distinct_not_implemented_checks(self):
114 114
         # distinct + aggregate not allowed
115 115
         with self.assertRaises(NotImplementedError):
116 116
             Celebrity.objects.distinct('id').aggregate(Max('id'))
  117
+
  118
+    def test_distinct_on_in_ordered_subquery(self):
  119
+        qs = Staff.objects.distinct('name').order_by('name', 'id')
  120
+        qs = Staff.objects.filter(pk__in=qs).order_by('name')
  121
+        self.assertQuerysetEqual(
  122
+            qs, [self.p1_o1, self.p2_o1, self.p3_o1],
  123
+            lambda x: x
  124
+        )
  125
+        qs = Staff.objects.distinct('name').order_by('name', '-id')
  126
+        qs = Staff.objects.filter(pk__in=qs).order_by('name')
  127
+        self.assertQuerysetEqual(
  128
+            qs, [self.p1_o2, self.p2_o1, self.p3_o1],
  129
+            lambda x: x
  130
+        )
16  tests/extra_regress/tests.py
@@ -347,3 +347,19 @@ def test_regression_17877(self):
347 347
             ['<TestObject: TestObject: a,a,a>', '<TestObject: TestObject: b,a,a>'],
348 348
             ordered=False
349 349
         )
  350
+
  351
+    def test_extra_values_distinct_ordering(self):
  352
+        t1 = TestObject.objects.create(first='a', second='a', third='a')
  353
+        t2 = TestObject.objects.create(first='a', second='b', third='b')
  354
+        qs = TestObject.objects.extra(
  355
+            select={'second_extra': 'second'}
  356
+        ).values_list('id', flat=True).distinct()
  357
+        self.assertQuerysetEqual(
  358
+            qs.order_by('second_extra'), [t1.pk, t2.pk], lambda x: x)
  359
+        self.assertQuerysetEqual(
  360
+            qs.order_by('-second_extra'), [t2.pk, t1.pk], lambda x: x)
  361
+        # Note: the extra ordering must appear in select clause, so we get two
  362
+        # non-distinct results here (this is on purpose, see #7070).
  363
+        self.assertQuerysetEqual(
  364
+            qs.order_by('-second_extra').values_list('first', flat=True),
  365
+            ['a', 'a'], lambda x: x)

0 notes on commit b1b04df

Please sign in to comment.
Something went wrong with that request. Please try again.