Opened 3 years ago

Last modified 12 days ago

#33450 assigned Bug

Integer primary key is wrongly casted to UUID when filtering GenericRelation on model with UUID primary key.

Reported by: Santos Gallegos Owned by: Clifford Gama
Component: Database layer (models, ORM) Version: 4.0
Severity: Normal Keywords:
Cc: Timur, Chinmoy Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi, at Read the Docs we have migrated from Django 2.2 to 3.2, and we encountered the following problem. In a model that makes use of a GenericForeignKey field, and it has a UUID field as its pk will result in Django casting the ID of the related model as UUID in some queries (the related model makes use of the default BigAutoField). I was able to reproduce this with the following models and using postgres as the DB backend.

from django.db import models
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes.fields import (
    GenericForeignKey,
    GenericRelation,
)
import uuid

class TTag(models.Model):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)

    content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
    object_id = models.PositiveIntegerField()
    content = GenericForeignKey()


class TIntegration(models.Model):
    tags = GenericRelation(TTag, related_query_name='integration')

The problematic queries:

integration = TIntegration.objects.create()
tag = integration.tags.create()
r = TTag.objects.filter(integration=integration)
print(r.query)

# The next error is raised if the query is evaluated
# psycopg2.errors.UndefinedFunction: operator does not exist: bigint = uuid

The query will result in the following SQL

SELECT "core_ttag"."id", "core_ttag"."content_type_id", "core_ttag"."object_id" FROM "core_ttag" INNER JOIN "core_tintegration" ON ("core_ttag"."object_id" = "core_tintegration"."id" AND ("core_ttag"."content_type_id" = 7)) WHERE "core_tintegration"."id" = 00000000-0000-0000-0000-000000000001

Note as core_tintegration"."id" is being casted as an UUID when it should be an integer (1). I was able to bisect the error to this commit https://github.com/django/django/commit/1afbc96a75bd1765a56054f57ea2d4b238af3f4d. Before that commit, the generated SQL is as follows:

SELECT "core_ttag"."id", "core_ttag"."content_type_id", "core_ttag"."object_id" FROM "core_ttag" INNER JOIN "core_tintegration" ON ("core_ttag"."object_id" = "core_tintegration"."id" AND ("core_ttag"."content_type_id" = 7)) WHERE "core_tintegration"."id" = 1

We were able to work around this issue by changing the queries to explicitly use the ID.

TTag.objects.filter(integration__id=integration.id)

According to the ticket's flags, the next step(s) to move this issue forward are:

  • For anyone except the patch author to review the patch using the patch review checklist and either mark the ticket as "Ready for checkin" if everything looks good, or leave comments for improvement and mark the ticket as "Patch needs improvement".

Change History (13)

comment:1 by Mariusz Felisiak, 3 years ago

Cc: Timur added
Summary: Integer PK wrongly casted as UUID when using with a GenericForeignKey model with a UUID fieldInteger primary key is wrongly casted to UUID when filtering GenericRelation on model with UUID primary key.
Triage Stage: UnreviewedAccepted

Thanks for the report! Regression in 1afbc96a75bd1765a56054f57ea2d4b238af3f4d.

While checking this report I found another issue that's related but it's not a regression:

>>> integration = TIntegration.objects.create()
>>> tag = integration.tags.create()
>>> tag.integration.first()
  ...
  File "/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: operator does not exist: uuid = numeric
LINE 1: ...ntent_type_id" = 7)) WHERE "test_33450_ttag"."id" = 10341161...

so cast to uuid is missing here 🤔.

comment:2 by Jeffrey, 3 years ago

Owner: changed from nobody to Jeffrey
Status: newassigned

comment:3 by Chinmoy, 3 years ago

Cc: Chinmoy added

comment:4 by Jeffrey, 3 years ago

Upon further investigation in Santos Gallegos original post, I also found that in RelatedLookupMixin.get_prep_lookup it also part of the issue since "target_field = self.lhs.output_field.path_infos[-1].fields[-1]" points at TTag's primary key which is a UUID instead of TIntegration's ID. Was able to resolve the issue by using reverse_path_infos but unsure if this is correct. So the issue is not so much related to https://github.com/django/django/commit/1afbc96a75bd1765a56054f57ea2d4b238af3f4d which does the casting but that at this point of the code it thinks it should get the ID value for TTag instead of TIntegration.

I will try to see if I can resolve the issue but wanted to share what I found so far. Let me know what you guys think.


comment:5 by Jeffrey, 3 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:6 by Jacob Walls, 3 years ago

Triage Stage: Ready for checkinAccepted

Thanks for submitting the PR. RFC is a status set by a reviewer.

comment:7 by Mariusz Felisiak, 3 years ago

Needs tests: set
Patch needs improvement: set

comment:8 by Abhinav Yadav, 3 years ago

Owner: changed from Jeffrey to Abhinav Yadav

comment:9 by Abhinav Yadav, 3 years ago

I have been working on this issue for a while, trying to understand it, but I'm unable to figure out how django is working internally to answer a question raised by Mariusz Felisiak in this comment asking why self.lhs.output_field points generic_relations.TIntegration.tags and not to generic_relations.TIntegration. I tried debugging and checking the variables in related_lookups.py but to no avail.

Any suggestions on where to proceed so as to fix this bug? The patch provided for this issue seems to work by reversing the fields but I suppose it's not the expected behaviour to begin with?

comment:10 by Clifford Gama, 3 weeks ago

Has patch: unset
Needs tests: unset
Owner: changed from Abhinav Yadav to Clifford Gama
Patch needs improvement: unset

comment:11 by Clifford Gama, 13 days ago

Has patch: set

comment:12 by Clifford Gama, 13 days ago

Patch needs improvement: set

comment:13 by Clifford Gama, 12 days ago

Patch needs improvement: unset
Note: See TracTickets for help on using tickets.
Back to Top