Opened 9 years ago

Closed 9 years ago

Last modified 6 years ago

#24343 closed Bug (fixed)

UUID foreign key accessor returns inconsistent type

Reported by: Tim Graham Owned by: nobody
Component: Database layer (models, ORM) Version: 1.8alpha1
Severity: Release blocker Keywords:
Cc: me@…, cmawebsite@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Current test failure on Oracle (and it's an issue on other backends but tests are skipped on other backends besides Oracle and PostgreSQL due to @skipUnlessDBFeature("can_defer_constraint_checks"):

Traceback (most recent call last):
  File "/mnt/jenkinsdata/workspace/django-oracle/database/oracle11/python/python2.7/django/utils/functional.py", line 15, in _curried
    return _curried_func(*(args + moreargs), **dict(kwargs, **morekwargs))
  File "/mnt/jenkinsdata/workspace/django-oracle/database/oracle11/python/python2.7/tests/serializers_regress/tests.py", line 479, in serializerTest
    func[1](self, pk, klass, datum)
  File "/mnt/jenkinsdata/workspace/django-oracle/database/oracle11/python/python2.7/tests/serializers_regress/tests.py", line 154, in fk_compare
    testcase.assertEqual(data, instance.data_id)
AssertionError: UUID('3f57e641-c3c7-4990-be67-f3207e4f5bd2') != u'3f57e641c3c74990be67f3207e4f5bd2'

Give this model:

class FKToUUID(models.Model):
    data = models.ForeignKey(UUIDData)

obj.data_id returns a string on all backends except PostgreSQL which returns a UUID. Might be related to the has_native_uuid_field database features flag.

Change History (9)

comment:1 by Tim Graham, 9 years ago

If the current behavior is correct, here's a patch to simply make the test pass:

diff --git a/tests/serializers_regress/tests.py b/tests/serializers_regress/tests.py
index e98a721..de2c0fc 100644
--- a/tests/serializers_regress/tests.py
+++ b/tests/serializers_regress/tests.py
@@ -151,6 +151,8 @@ def generic_compare(testcase, pk, klass, data):
 
 def fk_compare(testcase, pk, klass, data):
     instance = klass.objects.get(id=pk)
+    if isinstance(data, uuid.UUID) and not connection.features.has_native_uuid_field:
+        data = str(data).replace('-', '')
     testcase.assertEqual(data, instance.data_id

comment:2 by Shai Berger, 9 years ago

Is there a reason why we don't test the fk by accessing it?

That is, shouldn't we be doing something like

testcase.assertEqual(instance.data.pk, data)

?

Last edited 9 years ago by Shai Berger (previous) (diff)

comment:3 by Andriy Sokolovskiy, 9 years ago

Cc: me@… added

comment:4 by Collin Anderson, 9 years ago

Cc: cmawebsite@… added

comment:5 by Shai Berger, 9 years ago

For the people following this ticket: My suggestion above does not work, and is in the wrong direction. What appears to be the right direction is Marc's PR, following the little mailing list discussion.

comment:6 by Tim Graham, 9 years ago

Has patch: set

comment:7 by Marc Tamlyn <marc.tamlyn@…>, 9 years ago

Resolution: fixed
Status: newclosed

In 4755f8fc25331c739a6f932cc8aba0cc9e62e352:

Fixed #24343 -- Ensure db converters are used for foreign keys.

Joint effort between myself, Josh, Anssi and Shai.

comment:8 by Marc Tamlyn <marc.tamlyn@…>, 9 years ago

In c54d73ae01cd787315ba030da17e266c49f386b3:

[1.8.x] Fixed #24343 -- Ensure db converters are used for foreign keys.

Joint effort between myself, Josh, Anssi and Shai.

Conflicts:

django/db/models/query.py
tests/model_fields/models.py

Backport of 4755f8fc25331c739a6f932cc8aba0cc9e62e352 from master.

comment:9 by Tim Graham <timograham@…>, 6 years ago

In 5e3463f6:

Fixed #27595 -- Made ForeignKey.get_col() follow target chains.

Previously, foreign relationships were followed only one level deep which
prevents foreign keys to foreign keys from being resolved appropriately.
This was causing issues such as improper database value conversion for
UUIDField on SQLite because the resolved expression's output field's
internal type wasn't correct. Added tests to make sure unlikely foreign
reference cycles don't cause recursion errors.

Refs #24343.

Thanks oyooyo for the report and Wayne Merry for the investigation.

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