[Django] #24343: UUID foreign key accessor returns inconsistent type
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 1.8alpha1
layer (models, ORM) |
Severity: Release | Keywords:
blocker |
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/24343>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by shaib):
Is there a reason why we don't test the fk by accessing it?
That is, shouldn't we be doing something like
```python
testcase.assertEqual(instance.data.pk, data)
```
?
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: me@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: cmawebsite@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by shaib):
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
[https://github.com/django/django/pull/4134 Marc's PR], following the
little [https://groups.google.com/d/msgid/django-
developers/CAFO84S4CF_qo%2BMmJ1ybUsOMfD%3DPTyVxQZy8wD3vYKC6rE5%3DDwQ%40mail.gmail.com
mailing list discussion].
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"4755f8fc25331c739a6f932cc8aba0cc9e62e352"]:
{{{
#!CommitTicketReference repository=""
revision="4755f8fc25331c739a6f932cc8aba0cc9e62e352"
Fixed #24343 -- Ensure db converters are used for foreign keys.
Joint effort between myself, Josh, Anssi and Shai.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: timgraham | Owner: nobody
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Marc Tamlyn <marc.tamlyn@…>):
In [changeset:"c54d73ae01cd787315ba030da17e266c49f386b3"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:8>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.8alpha1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"5e3463f6bcec818431f0e1f4649d6a5bd944c459" 5e3463f6]:
{{{
#!CommitTicketReference repository=""
revision="5e3463f6bcec818431f0e1f4649d6a5bd944c459"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24343#comment:9>