Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#15203 closed Uncategorized (duplicate)

admin search_fields splitting query

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

Description (last modified by Ramiro Morales)

When you specify a ModelAdmin search_field as exact (with "=" prefix) the user's search query is split() and then multiple __iexact filters are applied, one for each word. This behavior doesn't make sense with a CharField because we're doing an iexact match (there's no way two distinct filters can match at the same time, except if we're doing a JOIN). It should just take the whole query and match against that.

The same problem probably also affects __search queries (with an "@" prefix). The string should probably also be passed directly to the DB.

Attachments (1)

admin-search.diff (1.5 KB ) - added by Waldemar Kornewald 13 years ago.
patch against trunk

Download all attachments as: .zip

Change History (14)

comment:1 by Waldemar Kornewald, 13 years ago

Cc: wkornewald@… added

comment:2 by Ramiro Morales, 13 years ago

Description: modified (diff)

(WikiFormatted slightly description)

comment:3 by Karen Tracey, 13 years ago

Needs documentation: set
Needs tests: set
Resolution: invalid
Status: newclosed

(When formatting only one inline backtick is needed; doubling them effectively removes them.)

I think you have overlooked the case of multiple fields listed in search_fields. I have used fields prefixed with equal in search_fields and I currently do get the results I expect, based on the existing doc, when entering multiple words in the search field. (One word entered can match exactly the equal-preceded field in search_fields and another can match some other field in search fields.) The doc already notes that it's not possible to search for an exact match on a value that contains a space. As far as I can see current behavior is easily understandable and documented, even if it is restrictive on what exactly you can search for. I think the change you propose would break the case where there are multiple fields in search_fields.

comment:4 by Waldemar Kornewald, 13 years ago

Resolution: invalid
Status: closedreopened

Right, I've overlooked that. But I don't agree that this ticket is invalid. Django's current behavior is not right. Splitting the string is very confusing and I'm pretty sure it's unnecessary in the case of __search. I'll fix the patch.

comment:5 by Waldemar Kornewald, 13 years ago

OK, I've updated the patch. The query string must still be split in case there are multiple fields, but additionally, exact matches on the whole query string should also be taken into account. In case of just a single search field the query should not be split.

The patch doesn't tread __search differently, anymore, since splitting is still necessary for multiple search fields, anyway.

The patch still doesn't have any documentation and unit tests. I'll try to push a complete patch in the next few weeks, but that'll probably be too late for the 1.3 release.

comment:6 by Karen Tracey, 13 years ago

Patch needs improvement: set

The updated patch still breaks the case I was concerned about above. When I've got multiple fields in search_fields, say:

    search_fields = ('=name', '=adopted_name', 'markings', 'adopter__name',)

(Happens to be a DB tracking cat adoptions for a rescue group.) Anyway, with current trunk code, if I enter "Lucky tabby" in the search box I get hits: cats with an (exact) name or adopted name of Lucky, and tabby somewhere in the markings field. This result I believe is correct, it matches my expectations and the documentation.

With the (updated) patch, I get no hits, which I don't believe is the correct result for this case.

by Waldemar Kornewald, 13 years ago

Attachment: admin-search.diff added

patch against trunk

comment:7 by Waldemar Kornewald, 13 years ago

Thanks for the note. That was caused by a typo. The new patch should work correctly, but I should really better write a few unit tests which check all important cases. ;)

comment:8 by anonymous, 13 years ago

Triage Stage: UnreviewedAccepted

comment:9 by Russell Keith-Magee, 13 years ago

Oops - that was me.

comment:10 by Waldemar Kornewald, 13 years ago

I was already wondering if the triage state needs to be reverted. :) Thanks for the note. FYI, I still haven't found the time to write the unit tests and update the documentation. Hopefully I'll finish this before you release 1.3 stable. :(

comment:11 by Julien Phalip, 13 years ago

Resolution: duplicate
Severity: Normal
Status: reopenedclosed
Type: Uncategorized

I think this is the same issue as #6933, which reports the problem when the field name starts with ^.

comment:12 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

comment:13 by Waldemar Kornewald, 12 years ago

Cc: wkornewald@… removed
Easy pickings: unset
UI/UX: unset
Note: See TracTickets for help on using tickets.
Back to Top