Opened 13 years ago

Closed 3 years ago

#16063 closed Cleanup/optimization (fixed)

Unnecessary joins in admin changelist query

Reported by: Evgeny Sizikov Owned by: Jacob Walls
Component: contrib.admin Version: 1.2
Severity: Normal Keywords:
Cc: goblinJoel Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Django 1.2.5

Models:

class Client(models.Model):
    name = models.CharField(_('name'), max_length=256)
    name2 = models.CharField(_('unofficial or obsolete name'), max_length=256, blank=True, null=True)
    contact_person = models.CharField(_('contact person'), max_length=256, blank=True, null=True)
    ...

class ClientOffice(models.Model):
    name = models.CharField(_('name'), max_length=256)
    name2 = models.CharField(_('unofficial or obsolete name'), max_length=256, blank=True, null=True)
    ...
    client = models.ForeignKey(Client, verbose_name=_('client'))
    ...

and admin options like these:

class ClientAdmin(admin.ModelAdmin):
    search_fields = ('name', 'name2', 'contact_person', 'clientoffice__name', 'clientoffice__name2')
    ...

Numbers:

>>> Client.objects.count()
10907
>>> ClientOffice.objects.count()
16952

Now, if we try searching for clients in admin by a search query containig several words (>3), got django/admin stalled.

The problem is going to be that each word in the search query leads to additional JOIN in final SQL query beacause of qs = qs.filter(...) pattern. The attached patch is for Django 1.2.5, but adopting for the current SVN trunk is trivial.

Attachments (1)

django_1_2_5_admin_search_m2m_prevent_multiple_joining.diff (943 bytes ) - added by Evgeny Sizikov 13 years ago.
patch has been made from Mercurial repository

Download all attachments as: .zip

Change History (16)

by Evgeny Sizikov, 13 years ago

patch has been made from Mercurial repository

comment:1 by Julien Phalip, 13 years ago

This seems related to #13902 and #15819. Are you able to test if this gets fixed by using Django 1.3 or trunk?

comment:2 by Aymeric Augustin, 13 years ago

Needs tests: set
Triage Stage: UnreviewedAccepted

Looking at the code, the part that is modified by the patch is not modified in current trunk. The problematic pattern is still there:

            for bit in self.query.split():
                ...
                qs = qs.filter(...)

comment:3 by Aymeric Augustin, 13 years ago

Type: BugCleanup/optimization

comment:4 by Evgeny Sizikov, 13 years ago

I have checked with Django 1.3 and can now confirm that the problem is still there. Django's ORM in 1.3 still generates SELECT with JOIN's for each qs = qs.filter(...) call, which is going to be made per term in a search query.

comment:5 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 by goblinJoel, 6 years ago

Cc: goblinJoel added

This is old, but I think it's a good idea. It's still a problem in Django 1.11, and I didn't see any changes in the 2.x code that looked relevant. It can end up devouring storage space larger than the db itself and suck up CPU to the point where the database system becomes unresponsive or runs out of storage.

I'm not sure whether I should be making unilateral edits, so I suggest renaming to something like:
Multi-word admin searches hang with foreign keys in search_fields

Related:

comment:7 by Jacob Walls, 3 years ago

Owner: changed from nobody to Jacob Walls
Status: newassigned

comment:8 by Jacob Walls, 3 years ago

Summary: Problem with searching in m2m fields in inherited modelUnnecessary joins in admin changelist query

comment:9 by Mariusz Felisiak, 3 years ago

#21241 was a duplicate, see for a related discussion.

comment:10 by Jacob Walls, 3 years ago

Needs tests: unset

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 948d6d88:

Refs #16063 -- Added tests for searching against multiple related fields in admin changelist.

comment:12 by Mariusz Felisiak, 3 years ago

Needs documentation: set
Patch needs improvement: set

comment:13 by Jacob Walls, 3 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:14 by Carlton Gibson, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 76ccce6:

Fixed #16063 -- Adjusted admin changelist searches spanning multi-valued relationships.

This reduces the likelihood of admin searches issuing queries with
excessive joins.

Note: See TracTickets for help on using tickets.
Back to Top