Commit 24ec9538 authored by Simon Charette's avatar Simon Charette
Browse files

Fixed #19195 -- Allow explicit ordering by a relation `_id` field.

Thanks to chrisedgemon for the report and shaib, akaariai and
timgraham for the review.
parent a5f6cbce
Loading
Loading
Loading
Loading
+3 −2
Original line number Diff line number Diff line
@@ -464,8 +464,9 @@ class SQLCompiler(object):
        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()
+27 −0
Original line number Diff line number Diff line
@@ -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
~~~~~~

+3 −0
Original line number Diff line number Diff line
@@ -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
^^^^^^^

+5 −11
Original line number Diff line number Diff line
@@ -17,25 +17,19 @@ from django.db import models
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
+75 −37
Original line number Diff line number Diff line
@@ -5,26 +5,29 @@ from operator import attrgetter

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 @@ class OrderingTests(TestCase):
            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 @@ class OrderingTests(TestCase):
            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 @@ class OrderingTests(TestCase):
            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 @@ class OrderingTests(TestCase):
            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 @@ class OrderingTests(TestCase):
            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 @@ class OrderingTests(TestCase):
            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 @@ class OrderingTests(TestCase):
            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 @@ class OrderingTests(TestCase):
        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",