Code

Opened 5 years ago

Closed 14 months ago

Last modified 14 months ago

#11295 closed Cleanup/optimization (fixed)

If ModelAdmin.queryset returns a filtered QS don't require a 2nd count call

Reported by: Alex Owned by: viciu
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There is an optimization that tests QuerySet.query.where to see if we can reuse a count call, however we don't apply this optimization aggressively enough.

Attachments (2)

admin-count.diff (1.1 KB) - added by Alex 5 years ago.
admin-count.2.diff (3.5 KB) - added by Alex 4 years ago.

Download all attachments as: .zip

Change History (12)

Changed 5 years ago by Alex

comment:1 Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 4 years ago by Alex

comment:2 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:3 Changed 3 years ago by patchhammer

  • Easy pickings unset
  • Patch needs improvement set

admin-count.2.diff fails to apply cleanly on to trunk

comment:4 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:5 Changed 14 months ago by viciu

  • Owner changed from Alex to viciu
  • Status changed from new to assigned

comment:6 Changed 14 months ago by viciu

I've updated patch so it applies correctly on current master.
Also, test in original patch assumes that there are 6 queries to db (including messages framework).

Currently, before applying patch there are 5 of them (including duplicated COUNT(*):

(Pdb) pprint(connection.queries)
[{'sql': 'QUERY = \'SELECT "django_session"."session_key", "django_session"."session_data", "django_session"."expire_date" FROM "django_session" WHERE ("django_session"."session_key" = %s  AND "django_session"."expire_date" > %s )\' - PARAMS = (\'2nj8364ycp90oge7xl5tf9mviz7grety\', \'2013-02-23 17:53:11.374277\')',
  'time': '0.000'},
 {'sql': 'QUERY = \'SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = %s \' - PARAMS = (100,)',
  'time': '0.001'},
 {'sql': 'QUERY = \'SELECT COUNT(*) FROM "admin_views_emptymodel" WHERE "admin_views_emptymodel"."id" > %s \' - PARAMS = (1,)',
  'time': '0.000'},
 {'sql': 'QUERY = \'SELECT COUNT(*) FROM "admin_views_emptymodel" WHERE "admin_views_emptymodel"."id" > %s \' - PARAMS = (1,)',
  'time': '0.000'},
 {'sql': 'QUERY = \'SELECT "admin_views_emptymodel"."id" FROM "admin_views_emptymodel" WHERE "admin_views_emptymodel"."id" > %s  ORDER BY "admin_views_emptymodel"."id" DESC\' - PARAMS = (1,)',
  'time': '0.000'}]

After patch was applied, COUNT(*) is executed once:

(Pdb) pprint(connection.queries)
[{'sql': 'QUERY = \'SELECT "django_session"."session_key", "django_session"."session_data", "django_session"."expire_date" FROM "django_session" WHERE ("django_session"."session_key" = %s  AND "django_session"."expire_date" > %s )\' - PARAMS = (\'5y65r95ng27kccdmkrtoo1qom7q4vhln\', \'2013-02-23 17:54:51.024189\')',
  'time': '0.000'},
 {'sql': 'QUERY = \'SELECT "auth_user"."id", "auth_user"."password", "auth_user"."last_login", "auth_user"."is_superuser", "auth_user"."username", "auth_user"."first_name", "auth_user"."last_name", "auth_user"."email", "auth_user"."is_staff", "auth_user"."is_active", "auth_user"."date_joined" FROM "auth_user" WHERE "auth_user"."id" = %s \' - PARAMS = (100,)',
  'time': '0.001'},
 {'sql': 'QUERY = \'SELECT COUNT(*) FROM "admin_views_emptymodel" WHERE "admin_views_emptymodel"."id" > %s \' - PARAMS = (1,)',
  'time': '0.000'},
 {'sql': 'QUERY = \'SELECT "admin_views_emptymodel"."id" FROM "admin_views_emptymodel" WHERE "admin_views_emptymodel"."id" > %s  ORDER BY "admin_views_emptymodel"."id" DESC\' - PARAMS = (1,)',
  'time': '0.000'}]

comment:7 Changed 14 months ago by viciu

New patch created.

Pull request here:

https://github.com/django/django/pull/820

comment:8 Changed 14 months ago by viciu

  • Patch needs improvement unset

comment:9 Changed 14 months ago by Wiktor Kolodziej <wiktor@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In f07a5f0a21857204465019b4e68f914d31cb396a:

Fixed #11295: If ModelAdmin.queryset returns a filtered QS don't require a 2nd count call

Original patch rewritten, added tests and get_filters_params method for ChangeList class.
Thanks Alex for the report.

comment:10 Changed 14 months ago by Honza Král <Honza.Kral@…>

In d7e835f76d81d66a6d8964dba20506a1c6e559d8:

Merge pull request #820 from viciu/11295

Fixed #11295: If ModelAdmin.queryset returns a filtered QS don't require a 2nd count call

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.