[Django] #34178: Prefetching a foreign key on GenericForeignKey results in incorrect queryset being selected
Django
queryset being selected
-------------------------------------+-------------------------------------
Reporter: Rohan | Owner: nobody
Nahata |
Type: Bug | Status: new
Component: Database | Version: 4.1
layer (models, ORM) | Keywords: GenericForeignKey,
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 |
-------------------------------------+-------------------------------------
These are the models that I created to test my hypothesis. .
{{{
class Foreign(models.Model):
value = models.CharField(max_length=20)
class OtherForeign(models.Model):
value = models.CharField(max_length=20)
class A(models.Model):
f = models.ForeignKey(Foreign, on_delete=models.PROTECT)
class B(models.Model):
f = models.ForeignKey(OtherForeign, on_delete=models.PROTECT)
class X(models.Model):
ct = models.ForeignKey(ContentType, on_delete=models.PROTECT)
object_id = models.IntegerField()
object = GenericForeignKey(ct_field='ct', fk_field='object_id')
}}}
The tables were populated with this sample data.
{{{
Foreign
id|value
1|A1
2|A2
3|A3
4|A4
5|A5
6|A6
7|A7
8|A8
9|A9
10|A10
Other Foreign
id|value
1|B1
2|B2
3|B3
4|B4
5|B5
6|B6
7|B7
8|B8
9|B9
10|B10
A
id|f_id
1|1
2|2
3|3
4|4
5|5
6|6
7|7
8|8
9|9
10|10
B
id|f_id
1|1
2|2
3|3
4|4
5|5
6|6
7|7
8|8
9|9
10|10
X
id|object_id|ct_id
1|1|11
2|2|11
3|3|11
4|4|11
5|5|11
6|6|11
7|7|11
8|8|11
9|9|11
10|10|11
11|1|10
12|2|10
13|3|10
14|4|10
15|5|10
16|6|10
17|7|10
18|8|10
19|9|10
20|10|10
}}}
Based on this data :
{{{
->> xx = X.objects.prefetch_related('object').all().order_by('id')
->> xx[0].object.f.value
'A1'
->> xx[10].object.f.value
'B1'
}}}
This behaviour is correct.
However, when we chain another prefetch to the same,
{{{
->> xx = X.objects.prefetch_related('object',
'object__f').all().order_by('id')
->> xx[0].object.f.value
'A1'
->> xx[10].object.f.value
'A1'
}}}
Here, {{{xx[10].object}}} returns the correct object, however, when
{{{xx[10].object.f}}} is evaluated it refers to model A instead of model
B. This leads me to believe that there is some sort of faulty caching
going on in the background that leads to model A being used.
--
Ticket URL: <https://code.djangoproject.com/ticket/34178>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
queryset being selected
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericForeignKey, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
Other Foreign
A
B
}}}
Based on this data :
This bug was tested on 4.1.3, 3.2.16, 2.2.28 and was reproducible.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34178#comment:1>
Django
queryset being selected
-------------------------------------+-------------------------------------
Reporter: Rohan Nahata | Owner: nobody
Component: Database layer | Version: 4.1
(models, ORM) |
Keywords: GenericForeignKey, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* type: Bug => New feature
* resolution: => duplicate
Comment:
`QuerySet.prefetch_related()` works with `GenericForeignKey`, however it
is only supported if the query is restricted to one `ContentType` (see
`prefetch_related()`
[https://docs.djangoproject.com/en/stable/ref/models/querysets/#prefetch-
related docs]).
Duplicate of #33651.
--
Ticket URL: <https://code.djangoproject.com/ticket/34178#comment:2>
Django
queryset being selected
-------------------------------------+-------------------------------------
Reporter: Rohan Nahata | Owner: nobody
Type: New feature | Status: closed
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: GenericForeignKey, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Rohan Nahata):
Replying to [comment:2 Mariusz Felisiak]:
> `QuerySet.prefetch_related()` works with `GenericForeignKey`, however it
is only supported if the query is restricted to one `ContentType` (see
`prefetch_related()`
[https://docs.djangoproject.com/en/stable/ref/models/querysets/#prefetch-
related docs]).
>
> Duplicate of #33651.
Just a question regarding this. Wouldn't this be the wrong behaviour
though? It silently does the wrong thing instead of erroring out.
--
Ticket URL: <https://code.djangoproject.com/ticket/34178#comment:3>
Django
queryset being selected
-------------------------------------+-------------------------------------
Reporter: Rohan Nahata | Owner: nobody
Type: New feature | Status: closed
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: GenericForeignKey, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
> It silently does the wrong thing instead of erroring out.
As far as I'm aware, raising an error would require fetching the content
type for each instance which is inefficient and may cause a performance
regression, see #21422.
--
Ticket URL: <https://code.djangoproject.com/ticket/34178#comment:4>
Django
queryset being selected
-------------------------------------+-------------------------------------
Reporter: Rohan Nahata | Owner: nobody
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution:
prefetch_related | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* type: New feature => Bug
* has_patch: 0 => 1
* resolution: duplicate =>
Comment:
We communicated with the author of #33651. He told us he was looking to
build a new functionality. Using his functionality, a developer will use
Prefetch functionality with GenericForeignkey. The ticket that @rohanahata
raised was -->we got the wrong data when we fired query as mentioned in
our ticket . We want to solve that bug . So our ticket is not a duplicate
of #33651. I request you to please reopen our ticket on these grounds. We
also have a patch for the same
--
Ticket URL: <https://code.djangoproject.com/ticket/34178#comment:5>
Django
queryset being selected
-------------------------------------+-------------------------------------
Reporter: Rohan Nahata | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: GenericForeignKey, | Triage Stage:
prefetch_related | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by MahavirAce):
Replying to [comment:4 Mariusz Felisiak]:
> > It silently does the wrong thing instead of erroring out.
>
> As far as I'm aware, raising an error would require fetching the content
type for each instance which is inefficient and may cause a performance
regression, see #21422.
We communicated with the author of #33651. He told us he was looking to
build a new functionality. Using his functionality, a developer will use
Prefetch functionality with GenericForeignkey. The ticket that @rohanahata
raised was -->we got the wrong data when we fired query as mentioned in
our ticket . We want to solve that bug . So our ticket is not a duplicate
of #33651. I request you to please reopen our ticket on these grounds. We
also have a patch for the same
--
Ticket URL: <https://code.djangoproject.com/ticket/34178#comment:6>
Django
queryset being selected
-------------------------------------+-------------------------------------
Reporter: Rohan Nahata | Owner: nobody
Component: Database layer | Version: 4.1
(models, ORM) |
Keywords: GenericForeignKey, | Triage Stage:
prefetch_related | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* has_patch: 1 => 0
* resolution: => duplicate
Comment:
Replying to [comment:5 MahavirAce]:
> We communicated with the author of #33651. He told us he was looking to
build a new functionality. Using his functionality, a developer will use
Prefetch functionality with GenericForeignkey. The ticket that @rohanahata
raised was -->we got the wrong data when we fired query as mentioned in
our ticket . We want to solve that bug . So our ticket is not a duplicate
of #33651. I request you to please reopen our ticket on these grounds. We
also have a patch for the same
#33651 is about exactly the same use case (see
[https://code.djangoproject.com/ticket/33651#comment:1 comment]) so using
`prefetch_related()` to `GenericForeignkey` with different content types.
We believe that a proper implementation here is to add the
`GenericPrefetch()` subclass. As I mentioned, the current behavior is
documented so we're treating this as a new feature.
> We also have a patch for the same .
You can submit the alternative patch to the #33651, if you believe that
`GenericPrefetch` is unnecessary and there is a better approach.