Re: [Django] #35309: Elide ordering of prefetch querysets for single valued relationships (was: Remove Order by on models when prefetching by id)

[Django]

#35309: Elide ordering of prefetch querysets for single valued relationships
-------------------------------------+-------------------------------------
Reporter: Laurent Lyaudet | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: prefetch single- | Triage Stage: Accepted
valued order_by |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* cc: Simon Charette (added)
* keywords: prefetch order_by => prefetch single-valued order_by
* needs_better_patch: 0 => 1
* resolution: wontfix =>
* stage: Unreviewed => Accepted
* status: closed => new
* summary: Remove Order by on models when prefetching by id => Elide
ordering of prefetch querysets for single valued relationships


Old description:

> Hello,
>
> I don't know if the "bug" is still here with Django 5.
> But on my version of Django, I have the following "bug" :
> Assume you have the following code :
>
> {{{#!python
> class A(models.Model):
> name = models.CharField(max_length=200)
>
> class Meta:
> ordering = ["name"]
>

> class B(models.Model):
> a = models.ForeignKey(A, related_name="bs", on_delete=models.CASCADE)
>

> a1 = A.objects.create(name="a1")
> a2 = A.objects.create(name="a2")
> a3 = A.objects.create(name="a3")
> a4 = A.objects.create(name="a4")
> a5 = A.objects.create(name="a5")
> a6 = A.objects.create(name="a6")
> a7 = A.objects.create(name="a7")
>
> b1 = B.objects.create(a=a1)
> b2 = B.objects.create(a=a2)
> b3 = B.objects.create(a=a3)
> b4 = B.objects.create(a=a4)
> b5 = B.objects.create(a=a5)
> b6 = B.objects.create(a=a6)
> b7 = B.objects.create(a=a7)
>
> bs = B.objects.all().prefetch_related("a")
> }}}
>
> The prefetch of as will use the order by and add useless charge on the DB
> server.
> There may be other cases than ForeignKey where the order by is useless.
> But since OneToOne inherits from ForeignKey, I don't see anything else
> right now.
>
> Hence, I request this enhancement, please :)
> #ClimateChangeBrake
>
> Best regards,
> Laurent Lyaudet

New description:

While the ordering of multi-valued relationships must be preserved when
prefetching relationships is it unnecessary when using `prefetch_related`
against single valued relationships.

For example, given the following models

{{{#!python
class Author(models.Model):
name = models.CharField(max_length=200)

class Meta:
ordering = ["name"]


class Book(models.Model):
title = models.CharField(max_length=200)
author = models.ForeignKey(Author, related_name="books",
on_delete=models.CASCADE)

class Meta:
ordering = ["title"]
}}}

The ordering of an author's books in
`Author.objects.prefetch_related("books")` has a significance as multiple
books might be associated with each authors.

It's not the case for a book's author in
`Book.objects.prefetch_related("author")` through as the relationship can
only contain a single author and there is a single way to order the
members of a singleton.

In other words `sorted([element], key=sort_func)` will result in
`[element]` for any `sort_func`.

This property holds true for all the single valued relationships that the
ORM supports (backward and forward 1:1 and forward 1:M) which allows the
prefetching to elide any predefined ordering safely to avoid an
unnecessary and possibly expensive ordering defined for the related model
queryset.

--
Comment:

I'm sorry for the awkward back and forth here but reviewing Laurent's PR
made something clear to me that wasn't from the origin report.

The requested optimization here is solely for single valued relationships
(backward and forward 1:1 and forward 1:M). In this scenario, as pointed
out by Laurent, `ORDER BY` doesn't matter as the related collection is
either empty or a singleton and thus `order_by()` can always be used in
their respective `get_prefetch_queryset`.

In the light of this realization I've adjusted the report and moved back
this ticket to an accepted optimization.

Laurent, as for the patch I suggest simply decorating existing tests that
make use of prefetching for single valued relationship (there are plenty
in `prefetch_related` tests`) which `assertNumQueries` and use the context
queries to assert against the lack of `ORDER BY`.

e.g.

{{{#!python
with self.assertNumQueries(2) as ctx:
list(Book.objects.prefetch_related("author"))
self.assertNotIn("ORDER BY", ctx.queries[-1]["sql"])
}}}

I think that systematically calling `order_by` without the
`_do_not_modify_order_by` should do.

Again, sorry for the misunderstanding and thank you for your efforts
towards contributing this improvement to Django.
--
Ticket URL: <https://code.djangoproject.com/ticket/35309#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.