Opened 7 years ago

Closed 7 years ago

#28852 closed Cleanup/optimization (wontfix)

Search in admin can't use index when several fields are in `search_fields`

Reported by: Jonathan Sundqvist Owned by: nobody
Component: contrib.admin Version: dev
Severity: Normal Keywords: search
Cc: Tomer Chachamu Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There are a couple of issues with the current implementation of search. If you have several fields like the following example.

    class MyAdmin(model.ModelAdmin):
        search_fields = ['=field_1', '@field_2']

You'll never be able to use any of the indexes that may exist on those fields, and if you were to remove field_1 you would still not be able to use any index on the full text search index for field_2.

If you have two or more fields in search fields you'll end up with a where statement that uses OR, and the index won't be used. In the case of a full text index you need to define the index with the appropriate config for the ts_vector. (See https://www.postgresql.org/docs/current/static/textsearch-tables.html#TEXTSEARCH-TABLES-INDEX).

The current implementation uses __search (https://github.com/django/django/blob/master/django/contrib/admin/options.py#L945) which ignores a specific config. So the index won't be used during the query.

To accomodate a config for a full text search I would suggest using @english@field_2. Then you could split on '@' and use a SearchVector and SearchQuery with the config english.

To make it possible to use index when several fields are defined in search_fields, instead of using OR. Make several querysets and do a union on all them. That way it will execute the query using the index.

Change History (3)

comment:1 by Tomer Chachamu, 7 years ago

Cc: Tomer Chachamu added

I've also had to customise ModelAdmin.get_search_results because of its not-so-smartness (I will probably open a separate ticket for it).

We've already moved away from the symbols for lookups, see https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.search_fields and previous discussion https://code.djangoproject.com/ticket/26184.

What do you think of overriding ModelAdmin.get_queryset to use .annotate(sv=SearchVector(...)) and then specifying 'sv__search' in the ModelAdmin.search_fields? It looks like that won't work with the current implementation, as search_fields must be model fields and not queryset annotations.

Last edited 7 years ago by Tomer Chachamu (previous) (diff)

comment:2 by Jonathan Sundqvist, 7 years ago

What do you think of overriding ModelAdmin.get_queryset to use .annotate(sv=SearchVector(...)) and then specifying 'svsearch' in the ModelAdmin.search_fields? It looks like that won't work with the current implementation, as search_fields must be model fields and not queryset annotations.

From what I can see that won't really work either even if you were to override the modeladmin queryset. Being able to set the config of the search vector and the search query is key to be able to use the index.

You could require a full_text_search setting in the modeladmin to configure the search vector.

full_text_search = {
    'config': 'english',
    'fields': ['field_1', 'field_2'],   
}

But as far as I know just using sv__search in that case would omit any config for that particular search query. I'll check it out and see what results I get.

Version 0, edited 7 years ago by Jonathan Sundqvist (next)

comment:3 by Tim Graham, 7 years ago

Resolution: wontfix
Status: newclosed

Rather than adding more syntactic sugar like @english@field_2, I think overriding ModelAdmin.get_search_results() may be the best approach. If you come up with a solution that you think is particularly elegant, I'd please raise it on the DevelopersMailingList to see if there's a consensus to add it to Django.

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