prefetch_related() for deleted GenericForeignKey is not consistent.

[Martin Svoboda]

prefetch_related called for GenericForeignKey sets content_type_id and object_id to None, if the foreign object doesn't exist. This behaviour is not documented.

GenericForignKey is often used for audit records, so it can keep links to non-existing objects. Probably prefetch_related shouldn't touch original values of object_id and content_type_id and only set content_object to None.

from django.contrib.auth.models import User
from django.contrib.contenttypes.fields import GenericForeignKey
from django.contrib.contenttypes.models import ContentType
from django.db import models


class TaggedItem(models.Model):
    tag = models.SlugField()
    content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
    object_id = models.PositiveIntegerField()
    content_object = GenericForeignKey('content_type', 'object_id')

# init data
guido = User.objects.create(username='Guido')
t = TaggedItem(content_object=guido, tag='test')
t.save()
guido.delete()

# get content_object normally
tags_1 = TaggedItem.objects.filter(tag='test')
tags_1[0].content_object  # returns None
tags_1[0].object_id  # returns 1
tags_1[0].content_type_id  # returns X

# use prefetch_related
tags_2 = TaggedItem.objects.filter(tag='test').prefetch_related("content_object")
tags_2[0].content_object  # returns None
tags_2[0].object_id  # returns None
tags_2[0].content_type_id # returns None

[Carlton Gibson]

Hi Martin. I do agree that's a little surprising. I'll accept as a Documentation issue on the content types app, but I'll cc Simon and Mariusz, in case they want to take it as a bug. Thanks.

[Simon Charette]

I didn't look at the issue in detail but I assume this is happening due to the prefetching logic performing a tags_2[0].content_object = None assignment.

How does the following code behaves?

tags_1 = TaggedItem.objects.filter(tag='test')
tags_1.content_object = None
assert tags_1.object_id is None
assert tags_1.content_type_id is None

[Martin Svoboda]

Thank you, you are right, assignment tags_2[0].content_object = None also set object_id and content_type_id is to None. However I am not sure if "modification" of a source object is the correct behaviour for prefetch_related.

Replying to Simon Charette:

I didn't look at the issue in detail but I assume this is happening due to the prefetching logic performing a tags_2[0].content_object = None assignment.

How does the following code behaves?

tags_1 = TaggedItem.objects.filter(tag='test')
tags_1.content_object = None
assert tags_1.object_id is None
assert tags_1.content_type_id is None

[Simon Charette]

Thanks for trying it out.

There must be a way to avoid this assignment overriding somehow as this analogous situation doesn't result in attribute loss

class UserRef(models.Model):
    user = models.ForeignKey(User, models.DO_NOTHING, null=True, db_constraint=False)

UserRef.objects.create(user_id=42)
ref = UserRef.objects.prefetch_related('user')[0]
assert ref.user is None
assert ref.user_id == 42

The distinction between the two is due to ​this branch where GenericForeignKey.get_prefetch_queryset sets is_descriptor to True.

I haven't tried it out but I suspect that switching is_descriptor to False ​instead now that GenericForeignKey has been changed to use the fields cache (bfb746f983aa741afa3709794e70f1e0ab6040b5) would address your issue and unify behaviours.

[Martin Svoboda]

Thank you, your recommendation actually fix the problem. I propose the patch ​https://github.com/django/django/pull/14762

I found one problem with the solution. prefetch_related sets tag.content_object to None, but access to this tag.content_object actually cause extra lookup query.

[Simon Charette]

Looks like the fetching of the object is due to the logic in __get__ ​https://github.com/django/django/blob/54a30a7a00fea6c5e3702282ade6e0238e06de3b/django/contrib/contenttypes/fields.py#L231-L245.

Not sure how feasible it would be to change it without breaking backward compatiblity though, you'd likely need to avoid passing default=None to get_cached_value and differentiate the sentinel value from None.

[Martin Svoboda]

I updated the object fetching logic of __get__. It respects None as value if the instance exists in the cache. Is this solution ok? It came to me as more readable than passing different None values.

​https://github.com/martinsvoboda/django/commit/7c83cec8b3bb99d9fe1822c3f82e58b373338a63#diff-76b33afd71c30afb749a0de312cfe4a4578d78abae84a9d5db06388a4b1bc454R231

[Mariusz Felisiak]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In cc4cb95b:

Fixed #33008 -- Fixed prefetch_related() for deleted GenericForeignKeys.

Thanks Simon Charette for the implementation idea.

[Mariusz Felisiak <felisiak.mariusz@…>]

In dd8945d3:

[4.0.x] Fixed #33008 -- Fixed prefetch_related() for deleted GenericForeignKeys.

Thanks Simon Charette for the implementation idea.

Backport of cc4cb95beff0b75ec169add7e94cc481624a41e6 from main