[Django] #34593: Django admin list does same Count query twice when there are no filters
Django
------------------------------------------------+--------------------------
Reporter: Alex | Owner: Alex
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
------------------------------------------------+--------------------------
The issue is due to this code:
{{{
# Get the number of objects, with admin filters applied.
result_count = paginator.count # First count() query
# Get the total number of objects, with no admin filters applied.
if self.model_admin.show_full_result_count:
full_result_count = self.root_queryset.count() # Second
count() query
else:
full_result_count = None
}}}
When the there are no filters, the same query is done for {{{ result_count
}}} and {{{ full_result_count }}} and the value will always be the same.
--
Ticket URL: <https://code.djangoproject.com/ticket/34593>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Alex | Owner: Alex
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 4.2
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by David Sanders):
* stage: Unreviewed => Accepted
Comment:
Thanks Alex,
I've assigned it to you as it sounded like you were interested in
submitting a PR?
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:1>
Django
--------------------------------------+------------------------------------
Reporter: Alex | Owner: Alex
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: Alex | Owner: Alex
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Alex):
PR: [https://github.com/django/django/pull/16891]
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:3>
Django
Reporter: Alex | Owner: Alex
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => wontfix
* stage: Accepted => Unreviewed
Comment:
Closing as "wontfix" per Simon's
[https://github.com/django/django/pull/16891#pullrequestreview-1440203134
comment].
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: Alex
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: 4.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"9d756afb07de8ef6e4d1980413979496643f1c3b" 9d756afb]:
{{{
#!CommitTicketReference repository=""
revision="9d756afb07de8ef6e4d1980413979496643f1c3b"
Refs #34593 -- Commented that the extra count is necessary in
ChangeList.get_results().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: Alex
Cleanup/optimization |
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* version: 4.2 => dev
* resolution: wontfix =>
Comment:
Bug reopened since I think I've found a solution that actually avoids the
extra count query when it's not needed.
PR: https://github.com/django/django/pull/17568
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: Alex
Type: | Status: new
Cleanup/optimization |
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: Alex
Cleanup/optimization |
Component: contrib.admin | Version: dev
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => wontfix
Comment:
I still have concerns. I appreciate you'd like to reopen the ticket, but
please first start a discussion on the DevelopersMailingList, where you'll
reach a wider audience and see what other think, and
[https://docs.djangoproject.com/en/stable/internals/contributing/triaging-
tickets/#closing-tickets follow triaging guidelines with regards to
wontfix tickets.]
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: Alex
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* cc: Simon Charette (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: Alex
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
I share Mariusz concerns, I'm not sure it's worth trying to be extra
clever here given the many hooks the admin exposes to configure returned
results and the fact `show_full_result_count = False` can disable the
extra count entirely in cases where it's expensive enough to consider this
option.
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Alex | Owner: Alex
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alex):
Replying to [comment:8 Mariusz Felisiak]:
> I still have concerns. I appreciate you'd like to reopen the ticket, but
please first start a discussion on the DevelopersMailingList, where you'll
reach a wider audience and see what other think, and
[https://docs.djangoproject.com/en/stable/internals/contributing/triaging-
tickets/#closing-tickets follow triaging guidelines with regards to
wontfix tickets.]
I didn't know of the mailing list. I continued the original discussion of
the forum when the bug was initially opened, which according to the
wontfix guidelines, it's one of the places where you can do it.
Next time I'll wait until I get more feedback (I didn't get much on the
forum) or try the mailing list.Thanks for the feedback.
https://forum.djangoproject.com/t/django-admin-list-does-count-query-
twice/21100
--
Ticket URL: <https://code.djangoproject.com/ticket/34593#comment:11>