[Django] #34251: Paginator should warn if ordering is not deterministic
Django
----------------------------------------------+------------------------
Reporter: Alexandr Tatarinov | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: 4.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
----------------------------------------------+------------------------
As documented in tickets #28657 and #28261, at least some database
backends will produce non-deterministic ordering when sorting by columns
with non-unique values.
This leads to inconsistent pagination, which I argue falls under the same
category as paginating by unordered QuerySet. To make things worse, this
one is easy to overlook, since non-unique columns like "name" can have
mostly unique values and you need to have duplicates at the page borders
to observe the issue, so it's more likely to happen in production
environments after operating fine for quite a while.
I suggest we add a warning similar to the UnorderedObjectListWarning when
total ordering as defined
[https://github.com/django/django/blob/main/django/contrib/admin/views/main.py#L390
in admin changelist] is not specified.
The outstanding question is, how to realistically deal with this situation
when ordering is provided by the end-user? It's typical to have i.e. "sort
by price asc/desc" option in the UI or API, which will result in a warning
since the price is not a unique column. Pushing responsibility to add "pk"
to each sorting option to the front end does not seem like a reasonable
way to me. This may be out of the scope of the current ticket, but I want
to make sure we have a plan. I would say we also shouldn't insert pk
automatically like ChangeList does, it'll at least break indexing as in
this ticket #29943. Would like to hear your thoughts.
--
Ticket URL: <https://code.djangoproject.com/ticket/34251>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Alexandr Tatarinov | Owner: nobody
Component: Core (Other) | Version: 4.1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => needsinfo
Comment:
Hi Alexandr.
> Would like to hear your thoughts.
> The outstanding question is, how to realistically deal with this
situation when ordering is provided by the end-user? It's typical to have
i.e. "sort by price asc/desc" option in the UI or API, which will result
in a warning since the price is not a unique column. Pushing
responsibility to add "pk" to each sorting option to the front end does
not seem like a reasonable way to me.
My initial thought is that you just wouldn't do this. You'd let the user
pick "sort by price asc/desc" sure, but then before passing that to the
ORM you'd add the needed "pk" to the ordering to give you the
deterministic ordering you need. (In general you're never going to pass
any user input to the ORM before passing it through a
sanitation/transformation layer, such as a form.)
Maybe this is a good idea. I don't know — what would it look like? (It's
really hard to say without a concrete suggestion in hand.) I think the way
forward would be to show what the edit to the paginator would look like —
is it just going to warn or do you think automatically enforcing the
correct ordering is wanted? With that in hand it's easier to say if it's
going be too complex or not to be worth the effort.
The flip side of complexity is how useful this might be. That it was fixed
for the admin is positive, see #17198 — ''...we can't expect users to
understand partial ordering...''. I still think a proof-of-concept would
be helpful, but a discussion on the DevelopersMailingList or the Django
Forum would reach a bigger audience, to get more voices.
Make sense?
--
Ticket URL: <https://code.djangoproject.com/ticket/34251#comment:1>
Django
------------------------------------+--------------------------------------
Reporter: Alexandr Tatarinov | Owner: nobody
Type: New feature | Status: closed
Component: Core (Other) | Version: 4.1
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------------+--------------------------------------
Comment (by Alexandr Tatarinov):
To clarify, this ticket only suggests adding a warning, but I wanted to
discuss the consequences.
I agree, let's see what the community thinks
https://groups.google.com/g/django-developers/c/fVZhDuiVfm8
--
Ticket URL: <https://code.djangoproject.com/ticket/34251#comment:2>
Django
------------------------------------+--------------------------------------
Reporter: Alexandr Tatarinov | Owner: nobody
Type: New feature | Status: closed
Component: Core (Other) | Version: 4.1
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------------+--------------------------------------
Comment (by Alexandr Tatarinov):
Related discussion for the Django REST Framework
https://github.com/encode/django-rest-framework/discussions/8840
--
Ticket URL: <https://code.djangoproject.com/ticket/34251#comment:3>