Skip to content

Prevented unnecessary distinct() call in SearchFilter. #3938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lovelydinosaur merged 2 commits into from
Jun 23, 2016
Merged

Prevented unnecessary distinct() call in SearchFilter. #3938

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ada749
Prevented unnecessary distinct() call in SearchFilter.
charettes Feb 16, 2016
93897b9
Refactored SearchFilter lookup prefixes.
charettes Feb 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Refactored SearchFilter lookup prefixes.
  • Loading branch information
@charettes
charettes committed Feb 16, 2016
commit 93897b90eef4e36b9964c449ee50f283ec309ad6
22 changes: 12 additions & 10 deletions rest_framework/filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ class SearchFilter(BaseFilterBackend):
# The URL query parameter used for the search.
search_param = api_settings.SEARCH_PARAM
template = 'rest_framework/filters/search.html'
lookup_prefixes = {
'^': 'istartswith',
'=': 'iexact',
'@': 'search',
'$': 'iregex',
}

def get_search_terms(self, request):
"""
Expand All @@ -143,23 +149,19 @@ def get_search_terms(self, request):
return params.replace(',', ' ').split()

def construct_search(self, field_name):
if field_name.startswith('^'):
return "%s__istartswith" % field_name[1:]
elif field_name.startswith('='):
return "%s__iexact" % field_name[1:]
elif field_name.startswith('@'):
return "%s__search" % field_name[1:]
if field_name.startswith('$'):
return "%s__iregex" % field_name[1:]
lookup = self.lookup_prefixes.get(field_name[0])
if lookup:
field_name = field_name[1:]
else:
return "%s__icontains" % field_name
lookup = 'icontains'
return LOOKUP_SEP.join([field_name, lookup])

def must_call_distinct(self, opts, lookups):
"""
Return True if 'distinct()' should be used to query the given lookups.
Copy link
Contributor

@lovelydinosaur lovelydinosaur Feb 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a link here to the Django WONTFIX ticket, or documentation, or similar?

Copy link
Contributor Author

@charettes charettes Feb 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's undergoing work to allow using subqueries to filter m2m but I'm unsure about which ticket you are referring to. This was mostly inspired by how the Django's admin handle a similar case.

Copy link
Contributor

@lovelydinosaur lovelydinosaur Feb 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about which ticket you are referring to.

No, I'm not sure either - I thought I'd remembered a django ticket referring to this that closed as WONTFIX, but I don't see anything like that now.

"""
for lookup in lookups:
if lookup[0] in {'^', '=', '@', '$'}:
if lookup[0] in self.lookup_prefixes:
lookup = lookup[1:]
parts = lookup.split(LOOKUP_SEP)
for part in parts:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ class SearchListView(generics.ListAPIView):

def test_must_call_distinct(self):
filter_ = filters.SearchFilter()
prefixes = ['', '^', '=', '@', '$']
prefixes = [''] + list(filter_.lookup_prefixes)
for prefix in prefixes:
self.assertFalse(
filter_.must_call_distinct(
Expand Down