Opened 9 years ago

Closed 7 weeks ago

Last modified 7 weeks ago

#26001 closed Cleanup/optimization (fixed)

Make ModelAdmin.search_fields raise data errors on __exact lookups for non-string fields.

Reported by: Tim Graham Owned by: Saurabh
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: 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

Currently, all queries are done as string lookups which gives something like this on PostgreSQL, for example: ("admin_views_pluggablesearchperson"."age"::text) = UPPER(20)). It would be more efficient if the admin cast the search value to an integer and used that for the query.

Change History (22)

comment:3 by Simon Charette, 9 years ago

I think we should favor the approach suggested in #26184 instead.

comment:4 by Tim Graham, 9 years ago

Patch needs improvement: set

I agree, let's revisit this if it doesn't.

comment:5 by Tim Graham, 6 years ago

Has patch: unset
Patch needs improvement: unset

As far as I tested, you can now use something like search_fields = ['votes__exact'] to do the appropriate query, however, if a non-integer search term is entered, the page crashes with ValueError invalid literal for int() with base 10: 'abc' from IntegerField.get_prep_value(). Probably that exception should be caught in ModelAdmin.get_search_results() and no results returned.

comment:6 by Nick Sandford, 6 years ago

Owner: changed from nobody to Nick Sandford
Status: newassigned

comment:7 by Nick Sandford, 6 years ago

Has patch: set

comment:8 by Tim Graham, 6 years ago

Patch needs improvement: set

in reply to:  5 comment:9 by Niclas Olofsson, 4 years ago

Replying to Tim Graham:

As far as I tested, you can now use something like search_fields = ['votes__exact'] to do the appropriate query, however, if a non-integer search term is entered, the page crashes with ValueError invalid literal for int() with base 10: 'abc' from IntegerField.get_prep_value(). Probably that exception should be caught in ModelAdmin.get_search_results() and no results returned.

We have to consider that there might be multiple search fields of mixed types though, i.e. search_fields = ['votes__exact', 'title', 'text__contains'], just so the matches for other fields does not get discarded just because one of the fields threw an error.

Instead of trying to catch these errors from .filter() as in the previous patch, we could run to_python() for each search field in order to test if that particular field would raise an exception later on when the query is run.

Last edited 4 years ago by Niclas Olofsson (previous) (diff)

comment:10 by Mariusz Felisiak, 2 years ago

Owner: Nick Sandford removed
Status: assignednew
Summary: Make ModelAdmin.search_fields do an integer lookup for IntegerFieldsMake ModelAdmin.search_fields raise data errors on __exact lookups for non-string fields.

comment:11 by Mariusz Felisiak, 2 years ago

#34191 was a duplicate for DecimalField.

in reply to:  11 comment:12 by Myhailo Chernyshov, 2 years ago

Replying to Mariusz Felisiak:

#34191 was a duplicate for DecimalField.

You can use iexact instead of exact for non-string fields, I think, it even makes sense, because the word exact means complete equality even with type. It works for me.

Last edited 2 years ago by Myhailo Chernyshov (previous) (diff)

comment:13 by Sandeep Pal, 11 months ago

can i work on this?

comment:14 by Saurabh , 11 months ago

Owner: set to Saurabh
Status: newassigned

comment:16 by John Beimler, 3 months ago

Patch needs improvement: unset

comment:17 by Sarah Boyce, 2 months ago

Patch needs improvement: set

comment:18 by Sarah Boyce, 2 months ago

Patch needs improvement: unset

comment:19 by Sarah Boyce, 7 weeks ago

Patch needs improvement: set

comment:20 by Sarah Boyce, 7 weeks ago

Patch needs improvement: unset

comment:21 by Sarah Boyce, 7 weeks ago

Triage Stage: AcceptedReady for checkin

comment:22 by Sarah Boyce <42296566+sarahboyce@…>, 7 weeks ago

Resolution: fixed
Status: assignedclosed

In f223729:

Fixed #26001 -- Fixed non-string field exact lookups in ModelAdmin.search_fields.

comment:23 by Sarah Boyce <42296566+sarahboyce@…>, 7 weeks ago

In 5fa4ccab:

Refs #26001 -- Handled relationship exact lookups in ModelAdmin.search_fields.

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