Ticket #21241: 881.patch

File 881.patch, 5.5 KB (added by Chris Adams, 11 years ago)

  • django/contrib/admin/options.py 

    From d4b88f068571cb5e5e4da48533b1f06aa2cf7618 Mon Sep 17 00:00:00 2001
From: Chris Adams <chris@improbable.org>
Date: Mon, 7 Oct 2013 10:48:19 -0400
Subject: [PATCH 1/2] Admin: optimize search filter construction

Using the database this way is inherently slow but the previous implementation would call QuerySet.filter() once per whitespace-separated term in the query text, causing an extra set of JOINs for each term. With the MySQL backend, this actually causes database errors once you hit the server limit of 61.

This commit logically ANDs each of the query components together so
`queryset.filter()` can be called a single time.
---
 django/contrib/admin/options.py | 10 +++++++---
 tests/admin_changelist/tests.py | 19 +++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 3b02ac0..f366b5d 100644
    a b def construct_search(field_name):  
    849849        if search_fields and search_term:
    850850            orm_lookups = [construct_search(str(search_field))
    851851                           for search_field in search_fields]
     852
     853            query_parts = []
    852854            for bit in search_term.split():
    853                 or_queries = [models.Q(**{orm_lookup: bit})
    854                               for orm_lookup in orm_lookups]
    855                 queryset = queryset.filter(reduce(operator.or_, or_queries))
     855                or_queries = (models.Q(**{orm_lookup: bit})
     856                              for orm_lookup in orm_lookups)
     857                query_parts.append(reduce(operator.or_, or_queries))
     858            queryset = queryset.filter(reduce(operator.and_, query_parts))
     859
    856860            if not use_distinct:
    857861                for search_spec in orm_lookups:
    858862                    if lookup_needs_distinct(self.opts, search_spec):

  • tests/admin_changelist/tests.py 

    diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py
index f9ef079..e49145f 100644
    a b def test_pagination_page_range(self):  
    643643                list(real_page_range),
    644644            )
    645645
     646    def test_search_query_efficiency(self):
     647        """Ensure that search queries only add one ORM filter rather than one per term"""
     648        new_parent = Parent.objects.create(name='parent')
     649        for i in range(200):
     650            Child.objects.create(name='foo bar baz qux quux corge %s' % i,
     651                                 parent=new_parent)
     652
     653        m = ParentAdmin(Parent, admin.site)
     654
     655        request = self.factory.get('/parent/', data={'q': 'foo bar baz'})
     656
     657        cl = ChangeList(request, Parent, m.list_display, m.list_display_links,
     658                        m.list_filter, m.date_hierarchy, m.search_fields,
     659                        m.list_select_related, m.list_per_page,
     660                        m.list_max_show_all, m.list_editable, m)
     661
     662        self.assertEqual(2, cl.queryset.query.count_active_tables(),
     663                         "ChangeList search filters should not cause duplicate JOINs")
     664
    646665
    647666class AdminLogNodeTestCase(TestCase):
    648667

  • tests/admin_changelist/tests.py 

    -- 
1.8.4


From 76ea5651266d0749701a238fb844333a6a0fc418 Mon Sep 17 00:00:00 2001
From: Chris Adams <chris@improbable.org>
Date: Mon, 7 Oct 2013 11:50:11 -0400
Subject: [PATCH 2/2] Tests: confirm that admin changelist search terms are
 ANDed

Performance work on #881 inadvertently demonstrated that there wasn't a
test for the documented behaviour of admin changelist terms being
evaluated as logical ANDs
---
 tests/admin_changelist/tests.py | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py
index e49145f..f63de41 100644
    a b def test_search_query_efficiency(self):  
    662662        self.assertEqual(2, cl.queryset.query.count_active_tables(),
    663663                         "ChangeList search filters should not cause duplicate JOINs")
    664664
     665    def test_search_query_logic(self):
     666        """Changelist search terms should be ANDed"""
     667
     668        parent1 = Parent.objects.create(name='parent 1')
     669        parent2 = Parent.objects.create(name='parent 2')
     670
     671        Child.objects.create(name='foo bar baz', parent=parent1)
     672        Child.objects.create(name='bar baz qux', parent=parent2)
     673
     674        m = ParentAdmin(Parent, admin.site)
     675
     676        request = self.factory.get('/parent/', data={'q': 'foo bar baz'})
     677
     678        cl = ChangeList(request, Parent, m.list_display, m.list_display_links,
     679                        m.list_filter, m.date_hierarchy, m.search_fields,
     680                        m.list_select_related, m.list_per_page,
     681                        m.list_max_show_all, m.list_editable, m)
     682
     683        cl.get_results(request)
     684        self.assertListEqual(["parent 1"], list(cl.queryset.values_list("name", flat=True)))
     685
     686
     687        request2 = self.factory.get('/parent/', data={'q': 'bar baz'})
     688        cl2 = ChangeList(request2, Parent, m.list_display, m.list_display_links,
     689                         m.list_filter, m.date_hierarchy, m.search_fields,
     690                         m.list_select_related, m.list_per_page,
     691                         m.list_max_show_all, m.list_editable, m)
     692        cl2.get_results(request2)
     693        self.assertListEqual(['parent 1', 'parent 2'],
     694                             list(cl2.queryset.order_by("name").values_list("name", flat=True)))
     695
    665696
    666697class AdminLogNodeTestCase(TestCase):
    667698
Back to Top