Opened 9 years ago

Closed 9 years ago

Last modified 5 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 Changed 9 years ago by Tim Graham

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 Changed 9 years ago by Shai Berger

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 Changed 9 years ago by Andriy Sokolovskiy

Cc: me@… added

comment:4 Changed 9 years ago by Collin Anderson

Cc: cmawebsite@… added

comment:5 Changed 9 years ago by Shai Berger

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 Changed 9 years ago by Tim Graham

Has patch: set

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

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 Changed 9 years ago by Marc Tamlyn <marc.tamlyn@…>

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 Changed 5 years ago by Tim Graham <timograham@…>

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