#26290 closed Cleanup/optimization (fixed)
Pagination module should warn about unordered query set
Reported by: | Kartik Anand | Owned by: | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | pagination |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Let's say somebody is using Gunicorn with more than one worker process.
Let's say contact_list is the following: (Paginating by 3)
[123, 456, 789, 345, 567, 909, 102]
When the first worker process handles request for page 1, it is possible that it gets the first three
[123, 456, 789],
but when the second worker process handles the request for page 2, it is very much possible that it returns,
[456, 789, 345]
See the duplicate results above. This is because the query set is unordered.
The above "may" also happen with just one worker process.
The documentation should warn about inconsistent pagination when no ordering is specified.
Change History (16)
comment:1 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Version: | 1.9 → master |
comment:2 by , 9 years ago
If we were to document or raise a warning in this case it must also be done when a queryset is ordered but not by any unique expression.
Else I don't see much benefit as ordering by a non unique expression can lead to the same undefined ordering reported here.
Since I can't think of any reliable way of determining if a queryset is uniquely ordered (matching ordered fields against unique
and unique_together
can yields false positive as users might have defined unique functional indexes using RunSQL
) I think we should favor a documentation patch.
comment:3 by , 9 years ago
Hmm I don't agree Simon. There's a vast difference between not ordered and mostly ordered. If a column is not unique you can still order by a second clause which will get you even closer to a unique ordering (and so on). Any duplicates in the final ordering will be randomly ordered (database dependent), but non duplicates will still be in the correct order. For most applications I would expect that to be OK.
Documentation should definitely be added. I would probably also like to see a warning about a completely unordered paginated queryset for the users sake.
comment:4 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
Was the runtime warning excluded somewhere? I still think it might be a good idea.
comment:9 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:10 by , 9 years ago
Component: | Documentation → Core (Other) |
---|---|
Has patch: | set |
Needs tests: | set |
PR (currently lacks a test)
comment:11 by , 9 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | set |
Left some comments for improvement and a few test failures remain.
comment:12 by , 9 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
De-assigning to make it clear that this is available for a PyCon sprinter to finish up.
comment:13 by , 9 years ago
Easy pickings: | unset |
---|
It looks like the patch was waiting for a review actually (which I've just done), so I'd give a chance to let the original owner finish it up.
Documentation is a minimum, I would also consider a runtime warning.