[Django] #32811: Annotate removes Meta.ordering
Django
-------------------------------------+-------------------------------------
Reporter: pirelle | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 3.2
layer (models, ORM) |
Severity: Release | Keywords:
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
This is very simplified example
{{{
In [5]: str(Order.objects.values("id", "ordered_at").all().query)
Out[5]: 'SELECT "orders_order"."id", "orders_order"."ordered_at" FROM
"orders_order" ORDER BY "orders_order"."ordered_at" DESC'
In [6]: str(Order.objects.values("id",
"ordered_at").annotate(c=Count("id")).all().query)
Out[6]: 'SELECT "orders_order"."id", "orders_order"."ordered_at",
COUNT("orders_order"."id") AS "c" FROM "orders_order" GROUP BY
"orders_order"."id"'
}}}
As you can see ORDER BY disappears when annotate with Count exists.
It used to work on 3.0, but stopped working in 3.1 and 3.2
--
Ticket URL: <https://code.djangoproject.com/ticket/32811>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: pirelle | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => invalid
* easy: 1 => 0
Comment:
This is an expected behavior. `Meta.ordering` doesn't affect `GROUP BY`
queries after 0ddb4ebf7bfcc4730c80a772dd146a49ef6895f6 (which was
deprecated in 1b1f64ee5a78cc217fead52cbae23114502cf564).
--
Ticket URL: <https://code.djangoproject.com/ticket/32811#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: pirelle | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Peter Law):
Request that this be reconsidered.
My use-case was similar to the one mentioned here, however I was loading
full models (with a similar count annotation) and not any form of
`.values()`. Given the history it seems likely that that was never the
intention.
I can see that this was previously deprecated and may now be expected,
however it does not appear to be documented anywhere and is very
surprising.
Here's where I've (since) checked for docs relating to this:
- https://docs.djangoproject.com/en/3.2/ref/models/options/#ordering
- https://docs.djangoproject.com/en/3.2/topics/db/aggregation/
- https://docs.djangoproject.com/en/3.2/ref/models/expressions/#aggregate-
expressions
What I've seen of the reasoning for the deprecation appears to be related
specifically to `.values()` queries, and then really to cases where that
is used to create a grouping ''of the main queried table'' (where the user
is probably aware that that's what they're doing). However the behaviour
change impacts ''all'' querysets (including those which return full
models) and the behaviour here is ''not'' intending to create a grouping
of things on the main table, but rather of a related table for the
purposes of counting (where the user is unlikely to be aware that a group-
by is the result).
Perhaps the core issue here is that the ORM doesn't have an explicit
"group by" mechanism and so is having to infer this by pattern matching
the structure of the `QuerySet`, leading to the issues like the original
one where default ordering is applied in ways that produce potentially
unexpected results.
At the very least I think this (current behaviour) needs to be clearly
documented as a part of both the `Meta.ordering` and aggregation docs,
however to be honest I would prefer the previous behaviour. It feels very
surprising that Django will silently drop part of its model configuration
in some cases!
At least if this were documented people could see what those cases were,
however at the moment the only way to know is when you're bitten by the
issue described here.
Aside: the original ticket (https://code.djangoproject.com/ticket/10574)
appears to suggest that part of the motivation is that in queries such as
`Model.objects.values('data')` applying the default ordering is redundant,
which I'd strongly argue against -- having values-only results ordered by
something (even a field not loaded) seems entirely useful to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/32811#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: pirelle | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:2 Peter Law]:
> Request that this be reconsidered.
>
> ...
> At the very least I think this (current behaviour) needs to be clearly
documented as a part of both the `Meta.ordering` and aggregation docs,
however to be honest I would prefer the previous behaviour. It feels very
surprising that Django will silently drop part of its model configuration
in some cases!
> At least if this were documented people could see what those cases were,
however at the moment the only way to know is when you're bitten by the
issue described here.
The previous behavior was implicit, unexpected, and caused many support
questions. We reached the consensus that removing any default ordering for
grouped queries is the right thing to do. You can always add `order_by()`
(as [https://docs.djangoproject.com/en/3.2/topics/db/aggregation
/#interaction-with-order-by documented]).
You can start a discussion on DevelopersMailingList if you don't agree,
but I don't think there would be consensus to restore it. Personally, I'm
strongly against it.
--
Ticket URL: <https://code.djangoproject.com/ticket/32811#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: pirelle | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Severity: Release blocker | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
-------------------------------------+-------------------------------------
Replying to [comment:2 Peter Law]:
full models (with a similar count annotation) and not any form of
`.values()`.
related specifically to `.values()` queries, and then really to cases
where that is used to create a grouping ''of the main queried table''
(where the user is probably aware that that's what they're doing). However
the behaviour change impacts ''all'' querysets (including those which
return full models) and the behaviour here is ''not'' intending to create
a grouping of things on the main table, but rather of a related table for
the purposes of counting (where the user is unlikely to be aware that a
group-by is the result).
as linked by Mariusz in section
[https://docs.djangoproject.com/en/5.2/topics/db/aggregation/#interaction-
with-order-by Interaction with order_by()], is a subsection of
[https://docs.djangoproject.com/en/5.2/topics/db/aggregation/#values
values()], suggesting that this is relevant only in the context when
`values()` and `order_by()` are used together. For me, too, it was not
obvious that this would apply to broader cases without `values()` as well.
Therefore, I would like to request that the documentation is clarified
about this.
--
Ticket URL: <https://code.djangoproject.com/ticket/32811#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: pirelle | Owner: nobody
Component: Database layer | Version: 3.2
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
-------------------------------------+-------------------------------------
* severity: Release blocker => Normal
--
Ticket URL: <https://code.djangoproject.com/ticket/32811#comment:5>