Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

[1.7.x] Fixed #19195 -- Allow explicit ordering by a relation `_id` f…

…ield.

Thanks to chrisedgemon for the report and shaib, akaariai and
timgraham for the review.

Backport of 24ec953 from master
  • Loading branch information...
commit a6ecd5dbb34249f756a337c359eef1e8d78dc01e 1 parent 1084456
@charettes charettes authored
View
5 django/db/models/sql/compiler.py
@@ -464,8 +464,9 @@ def find_ordering_name(self, name, opts, alias=None, default_order='ASC',
field, targets, alias, joins, path, opts = self._setup_joins(pieces, opts, alias)
# If we get to this point and the field is a relation to another model,
- # append the default ordering for that model.
- if field.rel and path and opts.ordering:
+ # append the default ordering for that model unless the attribute name
+ # of the field is specified.
+ if field.rel and path and opts.ordering and name != field.attname:
# Firstly, avoid infinite loops.
if not already_seen:
already_seen = set()
View
27 docs/ref/models/querysets.txt
@@ -294,6 +294,18 @@ primary key if there is no :attr:`Meta.ordering
...since the ``Blog`` model has no default ordering specified.
+.. versionadded:: 1.7
+
+ Note that it is also possible to order a queryset by a related field,
+ without incurring the cost of a JOIN, by referring to the ``_id`` of the
+ related field::
+
+ # No Join
+ Entry.objects.order_by('blog_id')
+
+ # Join
+ Entry.objects.order_by('blog__id')
+
Be cautious when ordering by fields in related models if you are also using
:meth:`distinct()`. See the note in :meth:`distinct` for an explanation of how
related model ordering can change the expected results.
@@ -435,6 +447,21 @@ Examples (those after the first will only work on PostgreSQL)::
>>> Entry.objects.order_by('author', 'pub_date').distinct('author')
[...]
+.. note::
+ Keep in mind that :meth:`order_by` uses any default related model ordering
+ that has been defined. You might have to explicitly order by the relation
+ ``_id`` or referenced field to make sure the ``DISTINCT ON`` expressions
+ match those at the beginning of the ``ORDER BY`` clause. For example, if
+ the ``Blog`` model defined an :attr:`~django.db.models.Options.ordering` by
+ ``name``::
+
+ Entry.objects.order_by('blog').distinct('blog')
+
+ ...wouldn't work because the query would be ordered by ``blog__name`` thus
+ mismatching the ``DISTINCT ON`` expression. You'd have to explicitly order
+ by the relation `_id` field (``blog_id`` in this case) or the referenced
+ one (``blog__pk``) to make sure both expressions match.
+
values
~~~~~~
View
3  docs/releases/1.7.txt
@@ -702,6 +702,9 @@ Models
Previously model field validation didn't prevent values out of their associated
column data type range from being saved resulting in an integrity error.
+* It is now possible to explicitly :meth:`~django.db.models.query.QuerySet.order_by`
+ a relation ``_id`` field by using its attribute name.
+
Signals
^^^^^^^
View
16 tests/ordering/models.py
@@ -17,25 +17,19 @@
from django.utils.encoding import python_2_unicode_compatible
-@python_2_unicode_compatible
-class Article(models.Model):
- headline = models.CharField(max_length=100)
- pub_date = models.DateTimeField()
-
+class Author(models.Model):
class Meta:
- ordering = ('-pub_date', 'headline')
-
- def __str__(self):
- return self.headline
+ ordering = ('-pk',)
@python_2_unicode_compatible
-class ArticlePKOrdering(models.Model):
+class Article(models.Model):
+ author = models.ForeignKey(Author, null=True)
headline = models.CharField(max_length=100)
pub_date = models.DateTimeField()
class Meta:
- ordering = ('-pk',)
+ ordering = ('-pub_date', 'headline')
def __str__(self):
return self.headline
View
112 tests/ordering/tests.py
@@ -5,26 +5,29 @@
from django.test import TestCase
-from .models import Article, ArticlePKOrdering
+from .models import Article, Author
class OrderingTests(TestCase):
- def test_basic(self):
- Article.objects.create(
+ def setUp(self):
+ self.a1 = Article.objects.create(
headline="Article 1", pub_date=datetime(2005, 7, 26)
)
- Article.objects.create(
+ self.a2 = Article.objects.create(
headline="Article 2", pub_date=datetime(2005, 7, 27)
)
- Article.objects.create(
+ self.a3 = Article.objects.create(
headline="Article 3", pub_date=datetime(2005, 7, 27)
)
- a4 = Article.objects.create(
+ self.a4 = Article.objects.create(
headline="Article 4", pub_date=datetime(2005, 7, 28)
)
- # By default, Article.objects.all() orders by pub_date descending, then
- # headline ascending.
+ def test_default_ordering(self):
+ """
+ By default, Article.objects.all() orders by pub_date descending, then
+ headline ascending.
+ """
self.assertQuerysetEqual(
Article.objects.all(), [
"Article 4",
@@ -35,8 +38,14 @@ def test_basic(self):
attrgetter("headline")
)
- # Override ordering with order_by, which is in the same format as the
- # ordering attribute in models.
+ # Getting a single item should work too:
+ self.assertEqual(Article.objects.all()[0], self.a4)
+
+ def test_default_ordering_override(self):
+ """
+ Override ordering with order_by, which is in the same format as the
+ ordering attribute in models.
+ """
self.assertQuerysetEqual(
Article.objects.order_by("headline"), [
"Article 1",
@@ -56,8 +65,11 @@ def test_basic(self):
attrgetter("headline")
)
- # Only the last order_by has any effect (since they each override any
- # previous ordering).
+ def test_order_by_override(self):
+ """
+ Only the last order_by has any effect (since they each override any
+ previous ordering).
+ """
self.assertQuerysetEqual(
Article.objects.order_by("id"), [
"Article 1",
@@ -77,7 +89,10 @@ def test_basic(self):
attrgetter("headline")
)
- # Use the 'stop' part of slicing notation to limit the results.
+ def test_stop_slicing(self):
+ """
+ Use the 'stop' part of slicing notation to limit the results.
+ """
self.assertQuerysetEqual(
Article.objects.order_by("headline")[:2], [
"Article 1",
@@ -86,8 +101,11 @@ def test_basic(self):
attrgetter("headline")
)
- # Use the 'stop' and 'start' parts of slicing notation to offset the
- # result list.
+ def test_stop_start_slicing(self):
+ """
+ Use the 'stop' and 'start' parts of slicing notation to offset the
+ result list.
+ """
self.assertQuerysetEqual(
Article.objects.order_by("headline")[1:3], [
"Article 2",
@@ -96,17 +114,20 @@ def test_basic(self):
attrgetter("headline")
)
- # Getting a single item should work too:
- self.assertEqual(Article.objects.all()[0], a4)
-
- # Use '?' to order randomly.
+ def test_random_ordering(self):
+ """
+ Use '?' to order randomly.
+ """
self.assertEqual(
len(list(Article.objects.order_by("?"))), 4
)
- # Ordering can be reversed using the reverse() method on a queryset.
- # This allows you to extract things like "the last two items" (reverse
- # and then take the first two).
+ def test_reversed_ordering(self):
+ """
+ Ordering can be reversed using the reverse() method on a queryset.
+ This allows you to extract things like "the last two items" (reverse
+ and then take the first two).
+ """
self.assertQuerysetEqual(
Article.objects.all().reverse()[:2], [
"Article 1",
@@ -115,7 +136,10 @@ def test_basic(self):
attrgetter("headline")
)
- # Ordering can be based on fields included from an 'extra' clause
+ def test_extra_ordering(self):
+ """
+ Ordering can be based on fields included from an 'extra' clause
+ """
self.assertQuerysetEqual(
Article.objects.extra(select={"foo": "pub_date"}, order_by=["foo", "headline"]), [
"Article 1",
@@ -126,8 +150,11 @@ def test_basic(self):
attrgetter("headline")
)
- # If the extra clause uses an SQL keyword for a name, it will be
- # protected by quoting.
+ def test_extra_ordering_quoting(self):
+ """
+ If the extra clause uses an SQL keyword for a name, it will be
+ protected by quoting.
+ """
self.assertQuerysetEqual(
Article.objects.extra(select={"order": "pub_date"}, order_by=["order", "headline"]), [
"Article 1",
@@ -143,21 +170,32 @@ def test_order_by_pk(self):
Ensure that 'pk' works as an ordering option in Meta.
Refs #8291.
"""
- ArticlePKOrdering.objects.create(
- pk=1, headline="Article 1", pub_date=datetime(2005, 7, 26)
- )
- ArticlePKOrdering.objects.create(
- pk=2, headline="Article 2", pub_date=datetime(2005, 7, 27)
- )
- ArticlePKOrdering.objects.create(
- pk=3, headline="Article 3", pub_date=datetime(2005, 7, 27)
- )
- ArticlePKOrdering.objects.create(
- pk=4, headline="Article 4", pub_date=datetime(2005, 7, 28)
+ Author.objects.create(pk=1)
+ Author.objects.create(pk=2)
+ Author.objects.create(pk=3)
+ Author.objects.create(pk=4)
+
+ self.assertQuerysetEqual(
+ Author.objects.all(), [
+ 4, 3, 2, 1
+ ],
+ attrgetter("pk")
)
+ def test_order_by_fk_attname(self):
+ """
+ Ensure that ordering by a foreign key by its attribute name prevents
+ the query from inheriting it's related model ordering option.
+ Refs #19195.
+ """
+ for i in range(1, 5):
+ author = Author.objects.create(pk=i)
+ article = getattr(self, "a%d" % (5 - i))
+ article.author = author
+ article.save(update_fields={'author'})
+
self.assertQuerysetEqual(
- ArticlePKOrdering.objects.all(), [
+ Article.objects.order_by('author_id'), [
"Article 4",
"Article 3",
"Article 2",

0 comments on commit a6ecd5d

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