From b1b04df06522b092a9b4768d2d61c52956e00eca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Fri, 11 Oct 2013 00:12:10 +0300 Subject: [PATCH] Fixed #20600 -- ordered distinct(*fields) in subqueries --- django/db/models/sql/compiler.py | 26 ++++++++++++++++---------- tests/distinct_on_fields/tests.py | 28 +++++++++++++++++++++------- tests/extra_regress/tests.py | 16 ++++++++++++++++ 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 230a8e19a28..38806e7dc20 100644 --- a/django/db/models/sql/compiler.py +++ b/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) - ordering_aliases.append(sql) - ordering_params.extend(self.query.extra[col][1]) + if must_append_to_select: + sql = "(%s) AS %s" % (self.query.extra[col][0], elt) + 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: - ordering_aliases.append(elt) - ordering_params.extend(params) - result.append('%s %s' % (elt, order)) + result.append('%s %s' % (elt, order)) group_by.append(self.query.extra[col]) self.ordering_aliases = ordering_aliases self.ordering_params = ordering_params diff --git a/tests/distinct_on_fields/tests.py b/tests/distinct_on_fields/tests.py index 9dedd98f81f..33ddf80ff43 100644 --- a/tests/distinct_on_fields/tests.py +++ b/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") - p2_o1 = Staff.objects.create(id=2, name="p2", organisation="o1") - p3_o1 = Staff.objects.create(id=3, name="p3", organisation="o1") - Staff.objects.create(id=4, name="p1", organisation="o2") - p1_o1.coworkers.add(p2_o1, p3_o1) - StaffTag.objects.create(staff=p1_o1, tag=t1) - StaffTag.objects.create(staff=p1_o1, tag=t1) + self.p1_o1 = Staff.objects.create(id=1, name="p1", organisation="o1") + self.p2_o1 = Staff.objects.create(id=2, name="p2", organisation="o1") + self.p3_o1 = Staff.objects.create(id=3, name="p3", organisation="o1") + self.p1_o2 = Staff.objects.create(id=4, name="p1", organisation="o2") + self.p1_o1.coworkers.add(self.p2_o1, self.p3_o1) + StaffTag.objects.create(staff=self.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 + ) diff --git a/tests/extra_regress/tests.py b/tests/extra_regress/tests.py index 3b40361750a..e9954add9bf 100644 --- a/tests/extra_regress/tests.py +++ b/tests/extra_regress/tests.py @@ -347,3 +347,19 @@ class ExtraRegressTests(TestCase): ['', ''], 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)