diff --git a/AUTHORS b/AUTHORS --- a/AUTHORS +++ b/AUTHORS @@ -544,16 +544,17 @@ ye7cakf02@sneakemail.com ymasuda@ethercube.com Jesse Young Mykola Zamkovoi zegor Gasper Zejn Jarek Zgoda Cheng Zhang + Jeffrey Gelens 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. diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -368,16 +368,19 @@ # 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,16 +524,27 @@ 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 diff --git a/django/db/backends/postgresql_psycopg2/base.py b/django/db/backends/postgresql_psycopg2/base.py --- a/django/db/backends/postgresql_psycopg2/base.py +++ b/django/db/backends/postgresql_psycopg2/base.py @@ -70,16 +70,17 @@ 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', diff --git a/django/db/backends/postgresql_psycopg2/operations.py b/django/db/backends/postgresql_psycopg2/operations.py --- a/django/db/backends/postgresql_psycopg2/operations.py +++ b/django/db/backends/postgresql_psycopg2/operations.py @@ -168,16 +168,24 @@ 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", () diff --git a/django/db/models/query.py b/django/db/models/query.py --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -693,22 +693,24 @@ """ 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,17 +1120,17 @@ 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): """ diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -71,18 +71,22 @@ 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) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -121,16 +121,17 @@ 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,16 +260,17 @@ 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,17 +389,17 @@ 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,16 +1592,25 @@ """ 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() diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -340,17 +340,17 @@ 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,16 +369,26 @@ 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. diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py --- a/tests/regressiontests/queries/models.py +++ b/tests/regressiontests/queries/models.py @@ -203,16 +203,19 @@ # 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,9 +340,23 @@ 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) diff --git a/tests/regressiontests/queries/tests.py b/tests/regressiontests/queries/tests.py --- a/tests/regressiontests/queries/tests.py +++ b/tests/regressiontests/queries/tests.py @@ -10,17 +10,18 @@ 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,16 +1732,31 @@ 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,16 +1818,66 @@ 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.objects.distinct('name').order_by('name'), + ['', '', ''], + ), + ( + Staff.objects.distinct('organisation').order_by('organisation', 'name'), + ['', ''], + ), + ( + Staff.objects.distinct('name', 'organisation').order_by('name', 'organisation'), + ['', '', '', ''], + ), + ( + Celebrity.objects.filter(fan__in=[self.fan1, self.fan2, self.fan3]).\ + distinct('name').order_by('name'), + ['', ''], + ), + ( + StaffTag.objects.distinct('staff','tag'), + [' p1>'], + ), + ( + Tag.objects.order_by('parent__pk').distinct('parent'), + ['', '', ''], + ), + ( + StaffTag.objects.select_related('staff').distinct('staff__name').order_by('staff__name'), + [' 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 = []