Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#24611 closed Bug (fixed)

Manager update with object instance having UUID as PK fails

Reported by: Jay Wineinger Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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 prepare_database_save(field) when a model instance is give as the query value. That 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.

Attachments (1)

update_uuid_fk.patch (2.7 KB) - added by Jay Wineinger 4 years ago.

Download all attachments as: .zip

Change History (5)

Changed 4 years ago by Jay Wineinger

Attachment: update_uuid_fk.patch added

comment:1 Changed 4 years ago by Tim Graham

Has patch: set
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

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.

comment:2 Changed 4 years ago by Jay Wineinger

Sure, here's a PR with tests moved and reusing existing test models. https://github.com/django/django/pull/4474

comment:3 Changed 4 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 923da02:

Fixed #24611 -- Fixed update() crash with related UUID pk object.

comment:4 Changed 4 years ago by Tim Graham <timograham@…>

In 496800b:

[1.8.x] Fixed #24611 -- Fixed update() crash with related UUID pk object.

Backport of 923da0274a9a8c4ef2ac9776f71bd14628e39fc3 from master

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