[Django] #30493: GenericRelation and prefetch_related: wrong caching with cyclic prefetching

20 views
Skip to first unread message

Django

unread,
May 20, 2019, 11:13:25 AM5/20/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic prefetching
-------------------------------------+-------------------------------------
Reporter: Finn | Owner: nobody
Stutzenstein |
Type: Bug | Status: new
Component: | Version: 2.2
contrib.contenttypes | Keywords: GenericRelation
Severity: Normal | prefetch_related
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Hello @all!

I encountered an issue with GenericRelations. Here is an example to
reproduce this issue:
https://github.com/FinnStutzenstein/GenericRelatedPrefetch
Just do a migrate and runserver. The code showing the error is started
automatically in main/apps.py. Please start with --noreload not to have
the output twice.

Whats the problem?
I have a generic model (Tag) that have a {{{content_object}}}. Then there
are multiple (in the example 2) models, Book and CD, that have exactly one
tag assigned. In the real application (OpenSlides) this invariant is
ensured in other places; in the example the objects are created in a way,
that this invariant holds.

All these content objects have a property {{{tag}}}, that should return
the one assigned tag. This is done by adding a helper field
{{{tags=GenericRelation(Tag)}}} and the property accesses
{{{self.tags.all()[0]}}}. The {{{.all()[0]}}} instead of a simple
{{{.get()}}} is required for the prefetching to work. See main/models.py
in the example.

Now all tags should be loaded because in OpenSlides they would be
serialized. For each tag the {{{content_object}}} is accessed as well as
{{{content_object.tag}}}. Because this would result in many DB queries (in
the real application about 10000 Tags, and in sum 10000 content objects)
the models are prefetched with:
{{{Tag.objects.prefetch_related("content_object",
"content_object__tag")}}} (Note: The executed code is in main/apps.py).
This results in a constant amount of queries (4 in this case) instead of
something proportional to the amount of objects. In the example you can
set {{{N}}}, the amount of objects created, to a higher amount to verify,
that the amount of queries stays constant.

What is expected: If I have a tag {{{tag}}}, {{{tag.content_object.tag}}}
should be equal to {{{tag}}}.
Output from the example (with N=1):

{{{
Got 'Tag to book0':
-the content object: Book0
-the content objects tag (should be the same as 'Tag to book0'!):Tag
to book0
Got 'Tag to cd0':
-the content object: CD0
-the content objects tag (should be the same as 'Tag to cd0'!):Tag to
book0
}}}

This is not the case: 'Tag to cd1' -> 'cd1' -> 'Tag to book1'.

I tracked this a bit showing, that {{{_prefetched_objects_cache}}} holds
the wrong value, which is accessed through {{{.all()}}} ->
{{{.get_queryset()}}} where the cached/prefetched result is taken.

Thanks!

--
Ticket URL: <https://code.djangoproject.com/ticket/30493>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
May 21, 2019, 5:01:56 AM5/21/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericRelation | Triage Stage: Accepted
prefetch_related |
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* version: 2.2 => master
* component: contrib.contenttypes => Database layer (models, ORM)
* stage: Unreviewed => Accepted


Comment:

Thanks for the report. This is definitely related with
`prefetch_related()`, because `get()` and `first()` works properly.

Reproduced at 519016e5f25d7c0a040015724f9920581551cab0.

--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:1>

Django

unread,
May 28, 2019, 3:51:30 AM5/28/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: nobody
Type: Bug | Status: new

Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericRelation | Triage Stage: Accepted
prefetch_related |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Can Sarıgöl):

* has_patch: 0 => 1


Comment:

[https://github.com/django/django/pull/11423 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:2>

Django

unread,
May 28, 2019, 9:31:23 AM5/28/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned

Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericRelation | Triage Stage: Accepted
prefetch_related |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Can Sarıgöl):

* status: new => assigned
* owner: nobody => Can Sarıgöl


--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:3>

Django

unread,
May 29, 2019, 5:41:28 AM5/29/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericRelation | Triage Stage: Accepted
prefetch_related |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* needs_better_patch: 0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:4>

Django

unread,
May 30, 2019, 7:52:47 AM5/30/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericRelation | Triage Stage: Ready for
prefetch_related | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:5>

Django

unread,
May 31, 2019, 1:06:23 AM5/31/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericRelation | Triage Stage: Accepted
prefetch_related |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):

* stage: Ready for checkin => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:6>

Django

unread,
May 31, 2019, 1:42:12 PM5/31/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: Can
| Sarıgöl
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericRelation | Triage Stage: Accepted
prefetch_related |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"f66021f3f773023d0c61d2537d5dd1e5e1740537" f66021f3]:
{{{
#!CommitTicketReference repository=""
revision="f66021f3f773023d0c61d2537d5dd1e5e1740537"
Refs #30493 -- Added GenericRelatedObjectManager.get_content_type() hook.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:7>

Django

unread,
May 31, 2019, 1:42:12 PM5/31/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: Can
| Sarıgöl
Type: Bug | Status: closed

Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed

Keywords: GenericRelation | Triage Stage: Accepted
prefetch_related |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"dffa3e1992562ba60512d96d1eb5859ffff2ceb5" dffa3e19]:
{{{
#!CommitTicketReference repository=""
revision="dffa3e1992562ba60512d96d1eb5859ffff2ceb5"
Fixed #30493 -- Fixed prefetch_related() for GenericRelation with
different content types.

Co-Authored-By: Mariusz Felisiak <felisiak...@gmail.com>

Thanks Simon Charette for the review.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:8>

Django

unread,
Jun 1, 2019, 6:49:02 AM6/1/19
to django-...@googlegroups.com
#30493: GenericRelation and prefetch_related: wrong caching with cyclic
prefetching.
-------------------------------------+-------------------------------------
Reporter: Finn Stutzenstein | Owner: Can
| Sarıgöl
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: GenericRelation | Triage Stage: Accepted
prefetch_related |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Finn Stutzenstein):

Thanks to Can Sarıgöl, felixxm and charettes for the fix! Great work!

--
Ticket URL: <https://code.djangoproject.com/ticket/30493#comment:9>

Reply all
Reply to author
Forward
0 new messages