From 670be13986b74f252d479ee2b7f74da8655273f6 Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Tue, 18 Mar 2008 10:21:50 +0000 Subject: [PATCH] queryset-refactor: Undo [7220] and allow ordering on multi-valued field. Some people will shoot themselves in the foot with this. That's bad luck. The reason we need it is because some data semantics cannot be expressed in Django's ORM and that shouldn't prevent ordering on that data. For example, filtering suburbs by a geographic region and then ordering on the suburb names. The names might not be unique outside that region, but unique inside it. Django cannot know that (you can't tell the model about it), so we trust the caller. git-svn-id: http://code.djangoproject.com/svn/django/branches/queryset-refactor@7285 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/sql/datastructures.py | 7 ++++++- django/db/models/sql/query.py | 15 ++++++--------- docs/db-api.txt | 13 +++++++------ tests/regressiontests/queries/models.py | 7 +++---- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/django/db/models/sql/datastructures.py b/django/db/models/sql/datastructures.py index bc21fb3b68..cb54a564f8 100644 --- a/django/db/models/sql/datastructures.py +++ b/django/db/models/sql/datastructures.py @@ -9,7 +9,12 @@ class EmptyResultSet(Exception): class FullResultSet(Exception): pass -class JoinError(Exception): +class MultiJoin(Exception): + """ + Used by join construction code to indicate the point at which a + multi-valued join was attempted (if the caller wants to treat that + exceptionally). + """ def __init__(self, level): self.level = level diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 771ee55e12..c153fe1e55 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -18,7 +18,7 @@ from django.db.models.sql.where import WhereNode, EverythingNode, AND, OR from django.db.models.sql.datastructures import Count from django.db.models.fields import FieldDoesNotExist from django.core.exceptions import FieldError -from datastructures import EmptyResultSet, Empty, JoinError +from datastructures import EmptyResultSet, Empty, MultiJoin from constants import * try: @@ -523,11 +523,8 @@ class Query(object): pieces = name.split(LOOKUP_SEP) if not alias: alias = self.get_initial_alias() - try: - field, target, opts, joins, last = self.setup_joins(pieces, opts, - alias, False, False) - except JoinError: - raise FieldError("Cannot order by many-valued field: '%s'" % name) + field, target, opts, joins, last = self.setup_joins(pieces, opts, + alias, False) alias = joins[-1] col = target.column @@ -848,7 +845,7 @@ class Query(object): try: field, target, opts, join_list, last = self.setup_joins(parts, opts, alias, (connector == AND), allow_many) - except JoinError, e: + except MultiJoin, e: self.split_exclude(filter_expr, LOOKUP_SEP.join(parts[:e.level])) return final = len(join_list) @@ -1007,7 +1004,7 @@ class Query(object): if not allow_many and (m2m or not direct): for alias in joins: self.unref_alias(alias) - raise JoinError(pos + 1) + raise MultiJoin(pos + 1) if model: # The field lives on a base class of the current model. alias_list = [] @@ -1175,7 +1172,7 @@ class Query(object): name.split(LOOKUP_SEP), opts, alias, False, allow_m2m, True) self.select.append((joins[-1], target.column)) - except JoinError: + except MultiJoin: raise FieldError("Invalid field name: '%s'" % name) def add_ordering(self, *ordering): diff --git a/docs/db-api.txt b/docs/db-api.txt index af5ba8ce49..e15b0b2176 100644 --- a/docs/db-api.txt +++ b/docs/db-api.txt @@ -510,12 +510,13 @@ primary key if there is no ``Meta.ordering`` specified. For example:: ...since the ``Blog`` model has no default ordering specified. -You can only order by model fields that have a single value attached to them -for each instance of the model. For example, non-relations, ``ForeignKey`` and -``OneToOneField`` fields. Explicitly, you can't order by a ``ManyToManyField`` -or a reverse ``ForeignKey`` relation. There's no naturally correct ordering -for many-valued fields and a lot of the alternatives are not psosible to -express in SQL very efficiently. +It is permissible to specify a multi-valued field to order the results by (for +example, a ``ManyToMany`` field). Normally this won't be a sensible thing to +do and it's really an advanced usage feature. However, if you know that your +queryset's filtering or available data implies that there will only be one +ordering piece of data for each of the main items you are selecting, the +ordering may well be exactly what you want to do. Use ordering on multi-valued +fields with care and make sure the results are what you expect. **New in Django development version:** If you don't want any ordering to be applied to a query, not even the default ordering, call ``order_by()`` with no diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py index a9783569a7..a9517628b4 100644 --- a/tests/regressiontests/queries/models.py +++ b/tests/regressiontests/queries/models.py @@ -424,11 +424,10 @@ FieldError: Infinite loop caused by ordering. [, , ] # Ordering by a many-valued attribute (e.g. a many-to-many or reverse -# ForeignKey) doesn't make sense (there's no natural ordering). +# ForeignKey) is legal, but the results might not make sense. That isn't +# Django's problem. Garbage in, garbage out. >>> Item.objects.all().order_by('tags') -Traceback (most recent call last): -... -FieldError: Cannot order by many-valued field: 'tags' +[...] # If we replace the default ordering, Django adjusts the required tables # automatically. Item normally requires a join with Note to do the default