Django

Code

Ticket #2818 (closed: fixed)

Opened 2 years ago

Last modified 2 years ago

[newforms-admin] Admin searches should use distinct, if query involves joins

Reported by: Andy Dustman <farcepest@gmail.com> Assigned to: adrian
Milestone: Component: django.contrib.admin
Version: SVN Keywords:
Cc: farcepest@gmail.com Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

I have a model that is searchable based on fields that are several levels of indirection away, i.e. 

class Course:

    # ...
    sections = ManyToManyField(Section)

    class Admin:

        search_fields = ('name', '^sections__instructors__surname',
                         '^sections__instructors__username',
                         '^sections__call_no')

i.e. Courses refers to Sections which refers to Instructors which has a couple fields to search on. Sections may belong to more than one course, and Instructors may belong to more than one section. In the current trunk, a search on surname in this scheme can return the same Course multiple times.

To fix this, I simply changed django.contrib.admin.views.main.ChangeList.get_query_set() to return a QuerySet().distinct() all the time. This doesn't seem to break anything and it fixes the duplicate results problem. However, it's possible it may have some performance impact so you might want to make it conditional on whether or not the search is across joined tables. However, it wasn't really obvious how to do this so I went for the quick-and-(clean|dirty) solution.

Attachments

search_distinct.diff (0.5 kB) - added by Andy Dustman <farcepest@gmail.com> on 09/25/06 15:45:53.
Make all admin searches distinct
search_distinct.2.diff (0.7 kB) - added by Andy Dustman <farcepest@gmail.com> on 10/10/06 15:16:48.
Use distinct() only when joined search fields are present

Change History

09/25/06 15:45:53 changed by Andy Dustman <farcepest@gmail.com>

  • attachment search_distinct.diff added.

Make all admin searches distinct

09/26/06 11:20:52 changed by adrian

  • summary changed from [patch] Admin searches should use distinct to Admin searches should use distinct, if query involves joins.

Yeah, I'm concerned about the performance impact that you pointed out. It should only add a DISTINCT if needed -- it should NOT always put that in.

I'm removing the [patch] designation.

10/10/06 15:16:48 changed by Andy Dustman <farcepest@gmail.com>

  • attachment search_distinct.2.diff added.

Use distinct() only when joined search fields are present

10/10/06 15:25:20 changed by anonymous

  • summary changed from Admin searches should use distinct, if query involves joins to [patch] Admin searches should use distinct, if query involves joins.

Requested changes made: As long as at least one field contains __, the resulting QuerySet will be made distinct().

One minor concern: I am explicitly looking for '__' in the field names and I wonder if I should look for LOOKUP_SEPARATOR from django.db.models.query. However a section above this (local function construct_search()) uses an explicit '__' and LOOKUP_SEPARATOR is not currently refererenced from outside of the module where it is defined, so I'm assuming it's OK.

04/21/07 08:28:10 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Ready for checkin.

04/27/07 03:25:20 changed by mtredinnick

  • summary changed from [patch] Admin searches should use distinct, if query involves joins to [newforms-admin] Admin searches should use distinct, if query involves joins.

04/28/07 10:45:36 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [5123]) newforms-admin: Fixed #2818 -- Filtered out duplicate results when searching in the admin across related fields. Thanks, Andy Dustman.

Add/Change #2818 ([newforms-admin] Admin searches should use distinct, if query involves joins)




Change Properties
Action