summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacob Walls <jacobtylerwalls@gmail.com>2021-10-05 21:38:15 -0400
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2021-12-15 08:14:19 +0100
commit76ccce64cc3d66cfec075651c3d2239fda747dc2 (patch)
tree6d0b0e141e8cc5affdb568aef63e3aaa7e2e9779
parente1d673c373a7d032060872b690a92fc95496612e (diff)
Fixed #16063 -- Adjusted admin changelist searches spanning multi-valued relationships.
This reduces the likelihood of admin searches issuing queries with excessive joins.
-rw-r--r--django/contrib/admin/options.py4
-rw-r--r--docs/ref/contrib/admin/index.txt32
-rw-r--r--docs/releases/4.1.txt20
-rw-r--r--docs/topics/db/queries.txt2
-rw-r--r--tests/admin_changelist/admin.py6
-rw-r--r--tests/admin_changelist/tests.py42
6 files changed, 102 insertions, 4 deletions
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 66f3396a6b..c5968a79ed 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -1031,6 +1031,7 @@ class ModelAdmin(BaseModelAdmin):
if search_fields and search_term:
orm_lookups = [construct_search(str(search_field))
for search_field in search_fields]
+ term_queries = []
for bit in smart_split(search_term):
if bit.startswith(('"', "'")) and bit[0] == bit[-1]:
bit = unescape_string_literal(bit)
@@ -1038,7 +1039,8 @@ class ModelAdmin(BaseModelAdmin):
*((orm_lookup, bit) for orm_lookup in orm_lookups),
_connector=models.Q.OR,
)
- queryset = queryset.filter(or_queries)
+ term_queries.append(or_queries)
+ queryset = queryset.filter(models.Q(*term_queries))
may_have_duplicates |= any(
lookup_spawns_duplicates(self.opts, search_spec)
for search_spec in orm_lookups
diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt
index 5caea603c1..ac8f44b765 100644
--- a/docs/ref/contrib/admin/index.txt
+++ b/docs/ref/contrib/admin/index.txt
@@ -1185,6 +1185,22 @@ subclass::
:meth:`ModelAdmin.get_search_results` to provide additional or alternate
search behavior.
+ .. versionchanged:: 4.1
+
+ Searches using multiple search terms are now applied in a single call
+ to ``filter()``, rather than in sequential ``filter()`` calls.
+
+ For multi-valued relationships, this means that rows from the related
+ model must match all terms rather than any term. For example, if
+ ``search_fields`` is set to ``['child__name', 'child__age']``, and a
+ user searches for ``'Jamal 17'``, parent rows will be returned only if
+ there is a relationship to some 17-year-old child named Jamal, rather
+ than also returning parents who merely have a younger or older child
+ named Jamal in addition to some other 17-year-old.
+
+ See the :ref:`spanning-multi-valued-relationships` topic for more
+ discussion of this difference.
+
.. attribute:: ModelAdmin.search_help_text
.. versionadded:: 4.0
@@ -1403,6 +1419,22 @@ templates used by the :class:`ModelAdmin` views:
field, for example ``... OR UPPER("polls_choice"."votes"::text) = UPPER('4')``
on PostgreSQL.
+ .. versionchanged:: 4.1
+
+ Searches using multiple search terms are now applied in a single call
+ to ``filter()``, rather than in sequential ``filter()`` calls.
+
+ For multi-valued relationships, this means that rows from the related
+ model must match all terms rather than any term. For example, if
+ ``search_fields`` is set to ``['child__name', 'child__age']``, and a
+ user searches for ``'Jamal 17'``, parent rows will be returned only if
+ there is a relationship to some 17-year-old child named Jamal, rather
+ than also returning parents who merely have a younger or older child
+ named Jamal in addition to some other 17-year-old.
+
+ See the :ref:`spanning-multi-valued-relationships` topic for more
+ discussion of this difference.
+
.. method:: ModelAdmin.save_related(request, form, formsets, change)
The ``save_related`` method is given the ``HttpRequest``, the parent
diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt
index 3fdd7f53c6..5f184d2349 100644
--- a/docs/releases/4.1.txt
+++ b/docs/releases/4.1.txt
@@ -290,6 +290,26 @@ Dropped support for MariaDB 10.2
Upstream support for MariaDB 10.2 ends in May 2022. Django 4.1 supports MariaDB
10.3 and higher.
+Admin changelist searches spanning multi-valued relationships changes
+---------------------------------------------------------------------
+
+Admin changelist searches using multiple search terms are now applied in a
+single call to ``filter()``, rather than in sequential ``filter()`` calls.
+
+For multi-valued relationships, this means that rows from the related model
+must match all terms rather than any term. For example, if ``search_fields``
+is set to ``['child__name', 'child__age']``, and a user searches for
+``'Jamal 17'``, parent rows will be returned only if there is a relationship to
+some 17-year-old child named Jamal, rather than also returning parents who
+merely have a younger or older child named Jamal in addition to some other
+17-year-old.
+
+See the :ref:`spanning-multi-valued-relationships` topic for more discussion of
+this difference. In Django 4.0 and earlier,
+:meth:`~django.contrib.admin.ModelAdmin.get_search_results` followed the
+second example query, but this undocumented behavior led to queries with
+excessive joins.
+
Miscellaneous
-------------
diff --git a/docs/topics/db/queries.txt b/docs/topics/db/queries.txt
index c24c06da68..eb68c7b4da 100644
--- a/docs/topics/db/queries.txt
+++ b/docs/topics/db/queries.txt
@@ -525,6 +525,8 @@ those latter objects, you could write::
Blog.objects.filter(entry__authors__isnull=False, entry__authors__name__isnull=True)
+.. _spanning-multi-valued-relationships:
+
Spanning multi-valued relationships
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/tests/admin_changelist/admin.py b/tests/admin_changelist/admin.py
index f8051f64fa..929539ea88 100644
--- a/tests/admin_changelist/admin.py
+++ b/tests/admin_changelist/admin.py
@@ -36,6 +36,12 @@ class ParentAdmin(admin.ModelAdmin):
list_select_related = ['child']
+class ParentAdminTwoSearchFields(admin.ModelAdmin):
+ list_filter = ['child__name']
+ search_fields = ['child__name', 'child__age']
+ list_select_related = ['child']
+
+
class ChildAdmin(admin.ModelAdmin):
list_display = ['name', 'parent']
list_per_page = 10
diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py
index 92ea0dd718..86fcab531a 100644
--- a/tests/admin_changelist/tests.py
+++ b/tests/admin_changelist/tests.py
@@ -30,8 +30,8 @@ from .admin import (
DynamicListDisplayLinksChildAdmin, DynamicListFilterChildAdmin,
DynamicSearchFieldsChildAdmin, EmptyValueChildAdmin, EventAdmin,
FilteredChildAdmin, GroupAdmin, InvitationAdmin,
- NoListDisplayLinksParentAdmin, ParentAdmin, QuartetAdmin, SwallowAdmin,
- site as custom_site,
+ NoListDisplayLinksParentAdmin, ParentAdmin, ParentAdminTwoSearchFields,
+ QuartetAdmin, SwallowAdmin, site as custom_site,
)
from .models import (
Band, CharPK, Child, ChordsBand, ChordsMusician, Concert, CustomIdUser,
@@ -153,6 +153,42 @@ class ChangeListTests(TestCase):
cl = ia.get_changelist_instance(request)
self.assertEqual(cl.queryset.query.select_related, {'player': {}, 'band': {}})
+ def test_many_search_terms(self):
+ parent = Parent.objects.create(name='Mary')
+ Child.objects.create(parent=parent, name='Danielle')
+ Child.objects.create(parent=parent, name='Daniel')
+
+ m = ParentAdmin(Parent, custom_site)
+ request = self.factory.get('/parent/', data={SEARCH_VAR: 'daniel ' * 80})
+ request.user = self.superuser
+
+ cl = m.get_changelist_instance(request)
+ with CaptureQueriesContext(connection) as context:
+ object_count = cl.queryset.count()
+ self.assertEqual(object_count, 1)
+ self.assertEqual(context.captured_queries[0]['sql'].count('JOIN'), 1)
+
+ def test_related_field_multiple_search_terms(self):
+ """
+ Searches over multi-valued relationships return rows from related
+ models only when all searched fields match that row.
+ """
+ parent = Parent.objects.create(name='Mary')
+ Child.objects.create(parent=parent, name='Danielle', age=18)
+ Child.objects.create(parent=parent, name='Daniel', age=19)
+
+ m = ParentAdminTwoSearchFields(Parent, custom_site)
+
+ request = self.factory.get('/parent/', data={SEARCH_VAR: 'danielle 19'})
+ request.user = self.superuser
+ cl = m.get_changelist_instance(request)
+ self.assertEqual(cl.queryset.count(), 0)
+
+ request = self.factory.get('/parent/', data={SEARCH_VAR: 'daniel 19'})
+ request.user = self.superuser
+ cl = m.get_changelist_instance(request)
+ self.assertEqual(cl.queryset.count(), 1)
+
def test_result_list_empty_changelist_value(self):
"""
Regression test for #14982: EMPTY_CHANGELIST_VALUE should be honored
@@ -555,7 +591,7 @@ class ChangeListTests(TestCase):
('Finlayson', 1),
('Finlayson Hype', 0),
('Jonathan Finlayson Duo', 1),
- ('Mary Jonathan Duo', 1),
+ ('Mary Jonathan Duo', 0),
('Oscar Finlayson Duo', 0),
):
with self.subTest(search_string=search_string):