Ticket #6422: distinct_on.5.diff

AUTHORS

@@ -544 +544 @@
     ye7cakf02@sneakemail.com
     ymasuda@ethercube.com
     Jesse Young <adunar@gmail.com>
     Mykola Zamkovoi <nickzam@gmail.com>
     zegor
     Gasper Zejn <zejn@kiberpipa.org>
     Jarek Zgoda <jarek.zgoda@gmail.com>
     Cheng Zhang
+    Jeffrey Gelens <jeffrey@gelens.org>
 
 A big THANK YOU goes to:
 
     Rob Curley and Ralph Gage for letting us open-source Django.
 
     Frank Wiles for making excellent arguments for open-sourcing, and for
     his sage sysadmin advice.
 

django/db/backends/__init__.py

@@ -368 +368 @@
 
     # Features that need to be confirmed at runtime
     # Cache whether the confirmation has been performed.
     _confirmed = False
     supports_transactions = None
     supports_stddev = None
     can_introspect_foreign_keys = None
 
+    # Support for the DISTINCT ON clause
+    can_distinct_on_fields = False
+
     def __init__(self, connection):
         self.connection = connection
 
     def confirm(self):
         "Perform manual checks of any database features that might vary between installs"
         self._confirmed = True
         self.supports_transactions = self._supports_transactions()
         self.supports_stddev = self._supports_stddev()
@@ -521 +524 @@
     def fulltext_search_sql(self, field_name):
         """
         Returns the SQL WHERE clause to use in order to perform a full-text
         search of the given field_name. Note that the resulting string should
         contain a '%s' placeholder for the value being searched against.
         """
         raise NotImplementedError('Full-text search is not implemented for this database backend')
 
+    def distinct(self, db_table, fields):
+        """
+        Returns an SQL DISTINCT clause which removes duplicate rows from the
+        result set. If any fields are given, only the given fields are being
+        checked for duplicates.
+        """
+        if fields:
+            raise NotImplementedError('DISTINCT ON fields is not supported by this database backend')
+        else:
+            return 'DISTINCT'
+
     def last_executed_query(self, cursor, sql, params):
         """
         Returns a string of the query last executed by the given cursor, with
         placeholders replaced with actual values.
 
         `sql` is the raw query containing placeholders, and `params` is the
         sequence of parameters. These are used by default, but this method
         exists for database backends to provide a better implementation

django/db/backends/postgresql_psycopg2/base.py

@@ -70 +70 @@
     needs_datetime_string_cast = False
     can_return_id_from_insert = True
     requires_rollback_on_dirty_transaction = True
     has_real_datatype = True
     can_defer_constraint_checks = True
     has_select_for_update = True
     has_select_for_update_nowait = True
     has_bulk_insert = True
+    can_distinct_on_fields = True
 
 
 class DatabaseWrapper(BaseDatabaseWrapper):
     vendor = 'postgresql'
     operators = {
         'exact': '= %s',
         'iexact': '= UPPER(%s)',
         'contains': 'LIKE %s',

django/db/backends/postgresql_psycopg2/operations.py

@@ -168 +168 @@
         macro in src/include/pg_config_manual.h .
 
         This implementation simply returns 63, but can easily be overridden by a
         custom database backend that inherits most of its behavior from this one.
         """
 
         return 63
 
+    def distinct(self, db_table, fields):
+        if fields:
+            table_name = self.quote_name(db_table)
+            fields = [table_name + "." + self.quote_name(field) for field in fields]
+            return 'DISTINCT ON (%s)' % ', '.join(fields)
+        else:
+            return 'DISTINCT'
+
     def last_executed_query(self, cursor, sql, params):
         # http://initd.org/psycopg/docs/cursor.html#cursor.query
         # The query attribute is a Psycopg extension to the DB API 2.0.
         return cursor.query
 
     def return_insert_id(self):
         return "RETURNING %s", ()
 

django/db/models/query.py

@@ -693 +693 @@
         """
         assert self.query.can_filter(), \
                 "Cannot reorder a query once a slice has been taken."
         obj = self._clone()
         obj.query.clear_ordering()
         obj.query.add_ordering(*field_names)
         return obj
 
-    def distinct(self, true_or_false=True):
+    def distinct(self, *field_names):
         """
         Returns a new QuerySet instance that will select only distinct results.
         """
         obj = self._clone()
-        obj.query.distinct = true_or_false
+        obj.query.add_distinct_fields(field_names)
+        obj.query.distinct = True
+
         return obj
 
     def extra(self, select=None, where=None, params=None, tables=None,
               order_by=None, select_params=None):
         """
         Adds extra SQL fragments to the query.
         """
         assert self.query.can_filter(), \
@@ -1118 +1120 @@
         return self
 
     def order_by(self, *field_names):
         """
         Always returns EmptyQuerySet.
         """
         return self
 
-    def distinct(self, true_or_false=True):
+    def distinct(self, fields=None):
         """
         Always returns EmptyQuerySet.
         """
         return self
 
     def extra(self, select=None, where=None, params=None, tables=None,
               order_by=None, select_params=None):
         """

django/db/models/sql/compiler.py

@@ -71 +71 @@
 
         where, w_params = self.query.where.as_sql(qn=qn, connection=self.connection)
         having, h_params = self.query.having.as_sql(qn=qn, connection=self.connection)
         params = []
         for val in self.query.extra_select.itervalues():
             params.extend(val[1])
 
         result = ['SELECT']
+
         if self.query.distinct:
-            result.append('DISTINCT')
+            distinct_sql = self.connection.ops.distinct(
+                self.query.model._meta.db_table, self.query.distinct_fields)
+            result.append(distinct_sql)
+
         result.append(', '.join(out_cols + self.query.ordering_aliases))
 
         result.append('FROM')
         result.extend(from_)
         params.extend(f_params)
 
         if where:
             result.append('WHERE %s' % where)

django/db/models/sql/query.py

@@ -121 +121 @@
         self.tables = []    # Aliases in the order they are created.
         self.where = where()
         self.where_class = where
         self.group_by = None
         self.having = where()
         self.order_by = []
         self.low_mark, self.high_mark = 0, None  # Used for offset/limit
         self.distinct = False
+        self.distinct_fields = None
         self.select_for_update = False
         self.select_for_update_nowait = False
         self.select_related = False
         self.related_select_cols = []
 
         # SQL aggregate-related attributes
         self.aggregates = SortedDict() # Maps alias -> SQL aggregate function
         self.aggregate_select_mask = None
@@ -259 +260 @@
         if self.group_by is None:
             obj.group_by = None
         else:
             obj.group_by = self.group_by[:]
         obj.having = copy.deepcopy(self.having, memo=memo)
         obj.order_by = self.order_by[:]
         obj.low_mark, obj.high_mark = self.low_mark, self.high_mark
         obj.distinct = self.distinct
+        obj.distinct_fields = self.distinct_fields
         obj.select_for_update = self.select_for_update
         obj.select_for_update_nowait = self.select_for_update_nowait
         obj.select_related = self.select_related
         obj.related_select_cols = []
         obj.aggregates = copy.deepcopy(self.aggregates, memo=memo)
         if self.aggregate_select_mask is None:
             obj.aggregate_select_mask = None
         else:
@@ -387 +389 @@
             in zip(query.aggregate_select.items(), result)
         ])
 
     def get_count(self, using):
         """
         Performs a COUNT() query using the current filter constraints.
         """
         obj = self.clone()
-        if len(self.select) > 1 or self.aggregate_select:
+        if len(self.select) > 1 or self.aggregate_select or (self.distinct and self.distinct_fields):
             # If a select clause exists, then the query has already started to
             # specify the columns that are to be returned.
             # In this case, we need to use a subquery to evaluate the count.
             from django.db.models.sql.subqueries import AggregateQuery
             subquery = obj
             subquery.clear_ordering(True)
             subquery.clear_limits()
 
@@ -1590 +1592 @@
         """
         Clears the list of fields to select (but not extra_select columns).
         Some queryset types completely replace any existing list of select
         columns.
         """
         self.select = []
         self.select_fields = []
 
+    def add_distinct_fields(self, field_names):
+        self.distinct_fields = []
+        options = self.get_meta()
+
+        for name in field_names:
+            field, source, opts, join_list, last, _ = self.setup_joins(
+                name.split(LOOKUP_SEP), options, self.get_initial_alias(), False)
+            self.distinct_fields.append(field.column)
+
     def add_fields(self, field_names, allow_m2m=True):
         """
         Adds the given (model) fields to the select set. The field names are
         added in the order specified.
         """
         alias = self.get_initial_alias()
         opts = self.get_meta()
 

docs/ref/models/querysets.txt

@@ -340 +340 @@
 a default ordering, or when using :meth:`order_by()`). If no such ordering is
 defined for a given ``QuerySet``, calling ``reverse()`` on it has no real
 effect (the ordering was undefined prior to calling ``reverse()``, and will
 remain undefined afterward).
 
 distinct
 ~~~~~~~~
 
-.. method:: distinct()
+.. method:: distinct(*fields)
 
 Returns a new ``QuerySet`` that uses ``SELECT DISTINCT`` in its SQL query. This
 eliminates duplicate rows from the query results.
 
 By default, a ``QuerySet`` will not eliminate duplicate rows. In practice, this
 is rarely a problem, because simple queries such as ``Blog.objects.all()``
 don't introduce the possibility of duplicate result rows. However, if your
 query spans multiple tables, it's possible to get duplicate results when a
@@ -369 +369 @@
     selected, the columns used in any :meth:`order_by()` (or default model
     ordering) will still be involved and may affect uniqueness of the results.
 
     The moral here is that if you are using ``distinct()`` be careful about
     ordering by related models. Similarly, when using ``distinct()`` and
     :meth:`values()` together, be careful when ordering by fields not in the
     :meth:`values()` call.
 
+.. versionadded:: 1.4
+   ``distinct()`` takes optional positional arguments ``*fields``, which specify
+   field names to which the ``DISTINCT`` should be limited. This translates to
+   a ``SELECT DISTINCT ON`` SQL query. Note that this ``DISTINCT ON`` query is
+   only available in PostgreSQL.
+
+.. note::
+    When optional ``*fields`` are given, you will have to add an :meth:`order_by`
+    call with the same field names as the leftmost arguments.
+
 values
 ~~~~~~
 
 .. method:: values(*fields)
 
 Returns a ``ValuesQuerySet`` — a ``QuerySet`` subclass that returns
 dictionaries when used as an iterable, rather than model-instance objects.
 

tests/regressiontests/queries/models.py

@@ -203 +203 @@
 
 # An inter-related setup with a model subclass that has a nullable
 # path to another model, and a return path from that model.
 
 class Celebrity(models.Model):
     name = models.CharField("Name", max_length=20)
     greatest_fan = models.ForeignKey("Fan", null=True, unique=True)
 
+    def __unicode__(self):
+        return self.name
+
 class TvChef(Celebrity):
     pass
 
 class Fan(models.Model):
     fan_of = models.ForeignKey(Celebrity)
 
 # Multiple foreign keys
 class LeafA(models.Model):
@@ -337 +340 @@
             return "category item: " + str(self.category)
 
 class OneToOneCategory(models.Model):
     new_name = models.CharField(max_length=15)
     category = models.OneToOneField(SimpleCategory)
 
     def __unicode__(self):
         return "one2one " + self.new_name
-    
+
+class Staff(models.Model):
+    name = models.CharField(max_length=50)
+    organisation = models.CharField(max_length=100)
+    tags = models.ManyToManyField(Tag, through='StaffTag')
+
+    def __unicode__(self):
+        return self.name
+
+class StaffTag(models.Model):
+    staff = models.ForeignKey(Staff)
+    tag = models.ForeignKey(Tag)
+
+    def __unicode__(self):
+        return u"%s -> %s" % (self.tag, self.staff)

tests/regressiontests/queries/tests.py

@@ -10 +10 @@
 from django.test import TestCase, skipUnlessDBFeature
 from django.utils import unittest
 from django.utils.datastructures import SortedDict
 
 from models import (Annotation, Article, Author, Celebrity, Child, Cover, Detail,
     DumbCategory, ExtraInfo, Fan, Item, LeafA, LoopX, LoopZ, 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)
+    ObjectC, CategoryItem, SimpleCategory, SpecialCategory, OneToOneCategory,
+    Staff, StaffTag)
 
 
 class BaseQuerysetTest(TestCase):
     def assertValueQuerysetEqual(self, qs, values):
         return self.assertQuerysetEqual(qs, values, transform=lambda x: x)
 
 
 class Queries1Tests(BaseQuerysetTest):
@@ -1731 +1732 @@
     def setUp(self):
         generic = NamedCategory.objects.create(name="Generic")
         t1 = Tag.objects.create(name='t1', category=generic)
         t2 = Tag.objects.create(name='t2', parent=t1, category=generic)
         t3 = Tag.objects.create(name='t3', parent=t1)
         t4 = Tag.objects.create(name='t4', parent=t3)
         t5 = Tag.objects.create(name='t5', parent=t3)
 
+        p1_o1 = Staff.objects.create(name="p1", organisation="o1")
+        p2_o1 = Staff.objects.create(name="p2", organisation="o1")
+        p3_o1 = Staff.objects.create(name="p3", organisation="o1")
+        p1_o2 = Staff.objects.create(name="p1", organisation="o2")
+
+        StaffTag.objects.create(staff=p1_o1, tag=t1)
+        StaffTag.objects.create(staff=p1_o1, tag=t1)
+
+        celeb1 = Celebrity.objects.create(name="c1")
+        celeb2 = Celebrity.objects.create(name="c2")
+
+        self.fan1 = Fan.objects.create(fan_of=celeb1)
+        self.fan2 = Fan.objects.create(fan_of=celeb1)
+        self.fan3 = Fan.objects.create(fan_of=celeb2)
+
     # In Python 2.6 beta releases, exceptions raised in __len__ are swallowed
     # (Python issue 1242657), so these cases return an empty list, rather than
     # raising an exception. Not a lot we can do about that, unfortunately, due to
     # the way Python handles list() calls internally. Thus, we skip the tests for
     # Python 2.6.
     @unittest.skipIf(sys.version_info[:2] == (2, 6), "Python version is 2.6")
     def test_infinite_loop(self):
         # If you're not careful, it's possible to introduce infinite loops via
@@ -1802 +1818 @@
             Number.objects.filter(num__in=numbers[:2000]).count(),
             2000
         )
         self.assertEqual(
             Number.objects.filter(num__in=numbers).count(),
             2500
         )
 
+    @skipUnlessDBFeature('can_distinct_on_fields')
+    def test_ticket6422(self):
+        # (qset, expected) tuples
+        qsets = (
+            (
+                Staff.objects.distinct().order_by('name'),
+                ['<Staff: p1>', '<Staff: p1>', '<Staff: p2>', '<Staff: p3>'],
+            ),
+            (
+                Staff.objects.distinct('name').order_by('name'),
+                ['<Staff: p1>', '<Staff: p2>', '<Staff: p3>'],
+            ),
+            (
+                Staff.objects.distinct('organisation').order_by('organisation', 'name'),
+                ['<Staff: p1>', '<Staff: p1>'],
+            ),
+            (
+                Staff.objects.distinct('name', 'organisation').order_by('name', 'organisation'),
+                ['<Staff: p1>', '<Staff: p1>', '<Staff: p2>', '<Staff: p3>'],
+            ),
+            (
+                Celebrity.objects.filter(fan__in=[self.fan1, self.fan2, self.fan3]).\
+                    distinct('name').order_by('name'),
+                ['<Celebrity: c1>', '<Celebrity: c2>'],
+            ),
+            (
+                StaffTag.objects.distinct('staff','tag'),
+                ['<StaffTag: t1 -> p1>'],
+            ),
+            (
+                Tag.objects.order_by('parent__pk').distinct('parent'),
+                ['<Tag: t3>', '<Tag: t5>', '<Tag: t1>'],
+            ),
+            (
+                StaffTag.objects.select_related('staff').distinct('staff__name').order_by('staff__name'),
+                ['<StaffTag: t1 -> p1'],
+            ),
+        )
+
+        for qset, expected in qsets:
+            self.assertQuerysetEqual(qset, expected)
+            self.assertEqual(qset.count(), len(expected))
+
+        # and check the fieldlookup
+        self.assertRaises(
+            FieldError,
+            lambda: Staff.objects.distinct('shrubbery')
+        )
+
+
 class UnionTests(unittest.TestCase):
     """
     Tests for the union of two querysets. Bug #12252.
     """
     def setUp(self):
         objectas = []
         objectbs = []
         objectcs = []