#18177 closed New feature (fixed)
Discovering a relation should cache the originator if applicable
Reported by: | Owned by: | Aymeric Augustin | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | related, cache, ForeignKey, OneToOneField, prefetch_related |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently, related fields will not cache the originator object (which holds the field) into the returned objects. This patch makes reverse ForeignKey and OneToOneField relations cache the originator(s) into the queryset, and then in turn to the returned objects.
In example, it validates this test, with Pool objects having a ForeignKey to Tournament objects:
def test_fk(self): with self.assertNumQueries(2): tournament = Tournament.objects.get(pk=1) pool = tournament.pool_set.all()[0] self.assertIs(tournament, pool.tournament)
Without this patch, the expression pool.tournament
would trigger a third query and create a different instance. It also works with .prefetch_related() calls.
Attachments (3)
Change History (10)
by , 13 years ago
Attachment: | prefetch_related_cache.patch added |
---|
comment:1 by , 13 years ago
Er, apparently, this was fixed while I was writing this patch for cases where prefetch_related was not used. The attached patch which applies against r17916 only adds the functionality for when prefetch_related is used. Imagine there's a .prefetch_related('pool_set')
on the third line of the example :)
def test_fk_prefetch_related(self): with self.assertNumQueries(2): tournament = ( Tournament.objects.prefetch_related('pool_set') .get(pk=1) ) pool = tournament.pool_set.all()[0] self.assertIs(tournament, pool.tournament)
by , 13 years ago
Attachment: | prefetch_related_cache_with_tests.patch added |
---|
Patch against r17916 (Forgot to add the tests first time around.)
comment:2 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 13 years ago
Owner: | changed from | to
---|
comment:4 by , 13 years ago
First, thanks for your work this patch! It was really helpful.
To be honest, I didn't really understand the changes at first sight, I used your tests (they are great) and I tried to get them to pass. The goal was to see if I'd end up with the same patch.
I quickly noticed that, given the internal API of prefetch_related
, this problem is difficult to fix. prefetch_one_level
will assign a single related object or a list of related objects in a cache. Unfortunately, accessing the cache directly bypasses the getters and setters of the relation descriptors. This gives us two options:
- emulate the appropriate bits of the setters in
get_prefetch_query_set()
; - change the API of
get_prefetch_query_set()
to return a setter function rather than the name of the cache attribute; then the setter could do whatever necessary.
The first option is the least invasive, and it led me to the patch I'm going to attach in a few minutes.
This patch does three things:
- It introduces a new attribute,
QuerySet._known_related_object
. This is only used to automatically set the reverse relation when creating objects through a related manager. When you're accessingtournament.pool_set.all()
,tournament.pool_set.all()._known_related_object == ('tournament', tournament)
, so each pool objects gets its 'tournament' attribute set to the originaltournament
instance. - It caches the reverse relation in several
get_prefetch_query_set()
methods, as explained above. - It cleans up a little bit the code of the
QuerySet
class.
by , 13 years ago
Attachment: | 18177.patch added |
---|
comment:5 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:6 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch