Opened 6 years ago

Closed 6 years ago

#30343 closed Bug (fixed)

Prefetch related is not working when used GFK for model that uses UUID field as PK.

Reported by: Timur Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: prefetch_related, UUID, generic foreign key
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

How to reproduce:

  • create model with UUID as primary key
class Foo(models.Model):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    ...
  • create another model with GFK to model Foo
class Bar(models.Model):
    foo_content_type = models.ForeignKey(
        ContentType, related_name='actor',
        on_delete=models.CASCADE, db_index=True
    )
    foo_object_id = models.CharField(max_length=255, db_index=True)
    foo = GenericForeignKey('foo_content_type', 'foo_object_id')
    ...
  • and try to get queryset with prefetch related (django orm engine return None for attribute foo):
Bar.objects.all().prefetch_related('foo')

Thanks a lot for your attention! Also i wanna point out some related bug report from third party library in which previously i faced with that issue, maybe it would useful – https://github.com/justquick/django-activity-stream/issues/245

Attachments (1)

30343-test.diff (2.3 KB ) - added by Mariusz Felisiak 6 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Mariusz Felisiak, 6 years ago

Resolution: wontfix
Status: newclosed
Version: 2.2master

Thanks for the report, however GenericForeignKey doesn't support prefetch_related(), from the same reason it doesn't not support also filter(), exclude(), etc. Please see documentation:

Due to the way GenericForeignKey is implemented, you cannot use such fields directly with filters (filter() and exclude(), for example) via the database API. Because a GenericForeignKey isn’t a normal field object, these examples will not work:
...

Provided example works as expected with UUID PK, e.g.:

>>> for bar in Bar.objects.all():
...    print(bar, bar.foo)
Bar object (4) Foo object (e6bdfd5e-51a8-49c7-9a7a-998452fad6aa)
Version 0, edited 6 years ago by Mariusz Felisiak (next)

comment:2 by Mariusz Felisiak, 6 years ago

Resolution: wontfix
Status: closednew
Triage Stage: UnreviewedAccepted

Sorry for previous comment.

This should work. Currently it works when foo_object_id is an UUIDField but doesn't work when it is a CharField.

by Mariusz Felisiak, 6 years ago

Attachment: 30343-test.diff added

comment:3 by Mariusz Felisiak, 6 years ago

IMO, adding get_prep_value() to the UUIDField should fix this issue 🤔:

  • django/db/models/fields/__init__.py

    diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py
    index 2307dcae25..0a7a6b9736 100644
    a b class UUIDField(Field):  
    23352335            return value
    23362336        return value.hex
    23372337
     2338    def get_prep_value(self, value):
     2339        value = super().get_prep_value(value)
     2340        if value is None:
     2341            return None
     2342        return self.to_python(value)
     2343
    23382344    def to_python(self, value):
    23392345        if value is not None and not isinstance(value, uuid.UUID):
    23402346            input_form = 'int' if isinstance(value, int) else 'hex'
Last edited 6 years ago by Mariusz Felisiak (previous) (diff)

comment:4 by Simon Charette, 6 years ago

Yeah that seems like the correct approach. I was about to suggest

  • django/contrib/contenttypes/fields.py

    diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py
    index ed98ecb48c..6098b71a39 100644
    a b class GenericForeignKey(FieldCacheMixin):  
    202202            else:
    203203                model = self.get_content_type(id=ct_id,
    204204                                              using=obj._state.db).model_class()
    205                 return (model._meta.pk.get_prep_value(getattr(obj, self.fk_field)),
     205                return (model._meta.pk.to_python(getattr(obj, self.fk_field)),
    206206                        model)

But it looks like this is really just an issue with UUIDField.

comment:5 by Mariusz Felisiak, 6 years ago

Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

comment:6 by Mariusz Felisiak, 6 years ago

Has patch: set

in reply to:  6 comment:7 by Timur, 6 years ago

Replying to felixxm: Thanks for your work!

PR

comment:8 by Simon Charette, 6 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by GitHub <noreply@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 1afbc96:

Fixed #30343 -- Fixed prefetch_related() for GenericForeignKey when PK of related field is UUIDField.

Note: See TracTickets for help on using tickets.
Back to Top