#34251 closed New feature (needsinfo)
Paginator should warn if ordering is not deterministic
Reported by: | Alexandr Tatarinov | Owned by: | nobody |
---|---|---|---|
Component: | Core (Other) | Version: | 4.1 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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 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.
Change History (3)
comment:1 by , 22 months ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:2 by , 22 months ago
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
comment:3 by , 22 months ago
Related discussion for the Django REST Framework
https://github.com/encode/django-rest-framework/discussions/8840
Hi Alexandr.
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... — but other tickets have been closed as wontfix/invalid, #28657 #28261 — …Closing issue due ordering by non-unique field…. 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?