Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#24702 closed Cleanup/optimization (needsinfo)

Cache Query objects when possible

Reported by: Omer Katz Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently Django constructs the Query objects every time you query the database.
This is often unnecessary and can be avoided.
I started memoizing some of the Query objects that the manager creates but I encountered some test failures related to prefetch_related querysets.
This is the memoized manager implementation https://github.com/thedrow/django/blob/feature/memoized-manager/django/db/models/manager.py#L244
I'm not sure it should be in a separate class but I created another one because of some of the required API changes.
Can anyone explain the test failures?

Change History (5)

comment:1 Changed 5 years ago by Omer Katz

These are the test failures:

======================================================================
ERROR: gis_tests.geoadmin.tests (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: gis_tests.geoadmin.tests
Traceback (most recent call last):
  File "/home/omer/.pyenv/versions/2.7.9/lib/python2.7/unittest/loader.py", line 254, in _find_tests
    module = self._get_module_from_name(name)
  File "/home/omer/.pyenv/versions/2.7.9/lib/python2.7/unittest/loader.py", line 232, in _get_module_from_name
    __import__(name)
  File "/home/omer/Documents/Projects/django/tests/gis_tests/geoadmin/tests.py", line 10, in <module>
    from .admin import UnmodifiableAdmin
  File "/home/omer/Documents/Projects/django/tests/gis_tests/geoadmin/admin.py", line 4, in <module>
    class UnmodifiableAdmin(admin.OSMGeoAdmin):
AttributeError: 'module' object has no attribute 'OSMGeoAdmin'


======================================================================
FAIL: test_traverse_single_item_property (prefetch_related.tests.CustomPrefetchTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/omer/Documents/Projects/django/tests/prefetch_related/tests.py", line 488, in test_traverse_single_item_property
    [['primary_house', 'occupants', 'houses']]
  File "/home/omer/Documents/Projects/django/django/test/testcases.py", line 91, in __exit__
    query['sql'] for query in self.captured_queries
AssertionError: 15 queries executed, 5 expected
Captured queries were:
QUERY = u'SELECT "prefetch_related_person"."id", "prefetch_related_person"."name" FROM "prefetch_related_person" ORDER BY "prefetch_related_person"."id" ASC' - PARAMS = ()
QUERY = u'SELECT ("prefetch_related_person_houses"."person_id") AS "_prefetch_related_val_person_id", "prefetch_related_house"."id", "prefetch_related_house"."name", "prefetch_related_house"."address", "prefetch_related_house"."owner_id", "prefetch_related_house"."main_room_id" FROM "prefetch_related_house" INNER JOIN "prefetch_related_person_houses" ON ( "prefetch_related_house"."id" = "prefetch_related_person_houses"."house_id" ) WHERE "prefetch_related_person_houses"."person_id" IN (%s, %s) ORDER BY "prefetch_related_house"."id" ASC' - PARAMS = (1, 2)
QUERY = u'SELECT "prefetch_related_room"."id", "prefetch_related_room"."name", "prefetch_related_room"."house_id" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" IN (%s, %s, %s, %s) ORDER BY "prefetch_related_room"."id" ASC' - PARAMS = (1, 2, 3, 4)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 1)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 2)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 1)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 2)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 3)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 4)
QUERY = u'SELECT ("prefetch_related_person_houses"."house_id") AS "_prefetch_related_val_house_id", "prefetch_related_person"."id", "prefetch_related_person"."name" FROM "prefetch_related_person" INNER JOIN "prefetch_related_person_houses" ON ( "prefetch_related_person"."id" = "prefetch_related_person_houses"."person_id" ) WHERE "prefetch_related_person_houses"."house_id" IN (%s, %s) ORDER BY "prefetch_related_person"."id" ASC' - PARAMS = (1, 3)
QUERY = u'SELECT ("prefetch_related_person_houses"."person_id") AS "_prefetch_related_val_person_id", "prefetch_related_house"."id", "prefetch_related_house"."name", "prefetch_related_house"."address", "prefetch_related_house"."owner_id", "prefetch_related_house"."main_room_id" FROM "prefetch_related_house" INNER JOIN "prefetch_related_person_houses" ON ( "prefetch_related_house"."id" = "prefetch_related_person_houses"."house_id" ) WHERE "prefetch_related_person_houses"."person_id" IN (%s, %s) ORDER BY "prefetch_related_house"."id" ASC' - PARAMS = (1, 2)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 1)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 2)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 3)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 4)

======================================================================
FAIL: test_order (prefetch_related.tests.LookupOrderingTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/omer/Documents/Projects/django/tests/prefetch_related/tests.py", line 922, in test_order
    [list(p.primary_house.occupants.all()) for p in qs]
  File "/home/omer/Documents/Projects/django/django/test/testcases.py", line 91, in __exit__
    query['sql'] for query in self.captured_queries
AssertionError: 14 queries executed, 4 expected
Captured queries were:
QUERY = u'SELECT "prefetch_related_person"."id", "prefetch_related_person"."name" FROM "prefetch_related_person" ORDER BY "prefetch_related_person"."id" ASC' - PARAMS = ()
QUERY = u'SELECT ("prefetch_related_person_houses"."person_id") AS "_prefetch_related_val_person_id", "prefetch_related_house"."id", "prefetch_related_house"."name", "prefetch_related_house"."address", "prefetch_related_house"."owner_id", "prefetch_related_house"."main_room_id" FROM "prefetch_related_house" INNER JOIN "prefetch_related_person_houses" ON ( "prefetch_related_house"."id" = "prefetch_related_person_houses"."house_id" ) WHERE "prefetch_related_person_houses"."person_id" IN (%s, %s) ORDER BY "prefetch_related_house"."id" ASC' - PARAMS = (1, 2)
QUERY = u'SELECT "prefetch_related_room"."id", "prefetch_related_room"."name", "prefetch_related_room"."house_id" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" IN (%s, %s, %s, %s) ORDER BY "prefetch_related_room"."id" ASC' - PARAMS = (1, 2, 3, 4)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 1)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 2)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 1)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 2)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 3)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 4)
QUERY = u'SELECT ("prefetch_related_person_houses"."house_id") AS "_prefetch_related_val_house_id", "prefetch_related_person"."id", "prefetch_related_person"."name" FROM "prefetch_related_person" INNER JOIN "prefetch_related_person_houses" ON ( "prefetch_related_person"."id" = "prefetch_related_person_houses"."person_id" ) WHERE "prefetch_related_person_houses"."house_id" IN (%s, %s) ORDER BY "prefetch_related_person"."id" ASC' - PARAMS = (1, 3)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 1)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 2)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 3)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_room" WHERE "prefetch_related_room"."house_id" = %s' - PARAMS = (u'*', 4)

======================================================================
FAIL: test_count (prefetch_related.tests.PrefetchRelatedTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/omer/Documents/Projects/django/tests/prefetch_related/tests.py", line 103, in test_count
    [b.first_time_authors.count() for b in qs]
  File "/home/omer/Documents/Projects/django/django/test/testcases.py", line 91, in __exit__
    query['sql'] for query in self.captured_queries
AssertionError: 6 queries executed, 2 expected
Captured queries were:
QUERY = u'SELECT "prefetch_related_book"."id", "prefetch_related_book"."title" FROM "prefetch_related_book" ORDER BY "prefetch_related_book"."id" ASC' - PARAMS = ()
QUERY = u'SELECT "prefetch_related_author"."id", "prefetch_related_author"."name", "prefetch_related_author"."first_book_id" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" IN (%s, %s, %s, %s) ORDER BY "prefetch_related_author"."id" ASC' - PARAMS = (1, 2, 3, 4)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" = %s' - PARAMS = (u'*', 1)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" = %s' - PARAMS = (u'*', 2)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" = %s' - PARAMS = (u'*', 3)
QUERY = u'SELECT COUNT(%s) AS "__count" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" = %s' - PARAMS = (u'*', 4)

======================================================================
FAIL: test_exists (prefetch_related.tests.PrefetchRelatedTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/omer/Documents/Projects/django/tests/prefetch_related/tests.py", line 108, in test_exists
    [b.first_time_authors.exists() for b in qs]
  File "/home/omer/Documents/Projects/django/django/test/testcases.py", line 91, in __exit__
    query['sql'] for query in self.captured_queries
AssertionError: 6 queries executed, 2 expected
Captured queries were:
QUERY = u'SELECT "prefetch_related_book"."id", "prefetch_related_book"."title" FROM "prefetch_related_book" ORDER BY "prefetch_related_book"."id" ASC' - PARAMS = ()
QUERY = u'SELECT "prefetch_related_author"."id", "prefetch_related_author"."name", "prefetch_related_author"."first_book_id" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" IN (%s, %s, %s, %s) ORDER BY "prefetch_related_author"."id" ASC' - PARAMS = (1, 2, 3, 4)
QUERY = u'SELECT (1) AS "a", "prefetch_related_author"."id", "prefetch_related_author"."name", "prefetch_related_author"."first_book_id" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" = %s ORDER BY "prefetch_related_author"."id" ASC' - PARAMS = (1,)
QUERY = u'SELECT (1) AS "a", "prefetch_related_author"."id", "prefetch_related_author"."name", "prefetch_related_author"."first_book_id" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" = %s ORDER BY "prefetch_related_author"."id" ASC' - PARAMS = (2,)
QUERY = u'SELECT (1) AS "a", "prefetch_related_author"."id", "prefetch_related_author"."name", "prefetch_related_author"."first_book_id" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" = %s ORDER BY "prefetch_related_author"."id" ASC' - PARAMS = (3,)
QUERY = u'SELECT (1) AS "a", "prefetch_related_author"."id", "prefetch_related_author"."name", "prefetch_related_author"."first_book_id" FROM "prefetch_related_author" WHERE "prefetch_related_author"."first_book_id" = %s ORDER BY "prefetch_related_author"."id" ASC' - PARAMS = (4,)

----------------------------------------------------------------------
Ran 9185 tests in 235.862s

FAILED (failures=4, errors=1, skipped=483, expected failures=8)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

comment:2 Changed 5 years ago by Shai Berger

Haven't looked in the tests code, so I can only guess that the tests fail because you're getting the memoized "all" query instead of one that should be limited by a FK (for the prefetched query).

This hints at a flaw in your design: You can't memoize based only on the call from the manager. You need to take account of how the manager was constructed. Also, on first reading your code, I thought it odd that you'd choose to put memoization on the manager class rather than the queryset class.

comment:3 Changed 5 years ago by Tim Graham

Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Version: 1.8master

comment:4 Changed 4 years ago by Tim Graham

Resolution: needsinfo
Status: newclosed

I guess it's not so clear how to proceed here. @thedrow, feel free to reopen if you return to this and get something working.

comment:5 Changed 4 years ago by Omer Katz

My approach doesn't work and I don't have an alternative.
I think this should go back to the drawing board.
Any thoughts on how this should be implemented?

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