Prefetching a foreign key on GenericForeignKey results in incorrect queryset being selected

[Rohan Nahata]

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.

This bug was tested on 4.1.3, 3.2.16, 2.2.28 and was reproducible.

[Mariusz Felisiak]

QuerySet.prefetch_related() works with GenericForeignKey, however it is only supported if the query is restricted to one ContentType (see prefetch_related() ​docs).

Duplicate of #33651.

[Rohan Nahata]

Replying to Mariusz Felisiak:

QuerySet.prefetch_related() works with GenericForeignKey, however it is only supported if the query is restricted to one ContentType (see 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.

[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.

[Mahavir Agrawal]

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

[Mariusz Felisiak]

Replying to 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 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.