[Django] #24611: Manager update with object instance having UUID as PK fails
Django
----------------------------------------------+--------------------
Reporter: jwineinger | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
I'm using UUIDs for the primary keys of some models. Other models relate
to those via FK. I ran into an issue today when trying to do a
RelatedModel.objects.update(fk=instance_with_uuid_pk), where SQLite failed
with
{{{
Traceback (most recent call last):
File "/src/django/tests/update/tests.py", line 154, in
test_update_uuid_fk_with_model_instance
UUIDRelation.objects.update(relation=self.u1)
File "/src/django/django/db/models/manager.py", line 127, in
manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/src/django/django/db/models/query.py", line 619, in update
rows = query.get_compiler(self.db).execute_sql(CURSOR)
File "/src/django/django/db/models/sql/compiler.py", line 1056, in
execute_sql
cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
File "/src/django/django/db/models/sql/compiler.py", line 835, in
execute_sql
cursor.execute(sql, params)
File "/src/django/django/db/backends/utils.py", line 64, in execute
return self.cursor.execute(sql, params)
File "/src/django/django/db/utils.py", line 95, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/src/django/django/utils/six.py", line 658, in reraise
raise value.with_traceback(tb)
File "/src/django/django/db/backends/utils.py", line 64, in execute
return self.cursor.execute(sql, params)
File "/src/django/django/db/backends/sqlite3/base.py", line 318, in
execute
return Database.Cursor.execute(self, query, params)
django.db.utils.InterfaceError: Error binding parameter 0 - probably
unsupported type.
}}}
Examining the SQLite table showed a CHAR32 column for the UUID and FK
fields, which seems appropriate since SQLite doesn't have a UUID column
like postgres. Then I traced execution of the query and found that the
params being sent to SQLite was still a UUID, not the hex representation.
Inserting the primary models and related models works fine, so it seems to
be an issue with updates. It looks like the SQLUpdateCompiler calls
[https://github.com/django/django/blob/dc27f3ee0c3eb9bb17d6cb764788eeaf73a371d7/django/db/models/sql/compiler.py#L1015
prepare_database_save(field)] when a model instance is give as the query
value. That
[https://github.com/django/django/blob/dc27f3ee0c3eb9bb17d6cb764788eeaf73a371d7/django/db/models/base.py#L881
just returns the value of the related field], which is just a UUID in this
case. This is passed along for execution, where SQLite pukes.
However, if you pass a UUID as the value (not a model instance),
field.get_db_prep_save is called which will convert the UUID to its hex
value if the DB doesn't support native UUIDs. Thus, updates given a raw
UUID work fine.
So it seems like all that needs to change is that we need to call
field.get_db_prep_save on the value returned from
val.prepare_database_save. I've attached a patch that does this.
--
Ticket URL: <https://code.djangoproject.com/ticket/24611>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jwineinger | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by jwineinger):
* Attachment "update_uuid_fk.patch" added.
Django
Reporter: jwineinger | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* severity: Normal => Release blocker
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
* has_patch: 0 => 1
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
Comment:
We might put the tests in `tests/model_fields/test_uuid.py` so we can
reuse existing models. Are you able to send a pull request? This also
seems to fix #24608.
--
Ticket URL: <https://code.djangoproject.com/ticket/24611#comment:1>
Django
Reporter: jwineinger | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by jwineinger):
Sure, here's a PR with tests moved and reusing existing test models.
https://github.com/django/django/pull/4474
--
Ticket URL: <https://code.djangoproject.com/ticket/24611#comment:2>
Django
Reporter: jwineinger | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"923da0274a9a8c4ef2ac9776f71bd14628e39fc3" 923da02]:
{{{
#!CommitTicketReference repository=""
revision="923da0274a9a8c4ef2ac9776f71bd14628e39fc3"
Fixed #24611 -- Fixed update() crash with related UUID pk object.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24611#comment:3>
Django
Reporter: jwineinger | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"496800b3bf4460e697dee942696eaee5c56f87a0" 496800b]:
{{{
#!CommitTicketReference repository=""
revision="496800b3bf4460e697dee942696eaee5c56f87a0"
[1.8.x] Fixed #24611 -- Fixed update() crash with related UUID pk object.
Backport of 923da0274a9a8c4ef2ac9776f71bd14628e39fc3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24611#comment:4>