Opened 10 months ago
Closed 10 months ago
#36086 closed Bug (fixed)
Creating an instance of a model with a non-auto primary key and a generated field crashes on backends not supporting returning columns from inserts
| Reported by: | Simon Charette | Owned by: | Simon Charette |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 5.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Discovered while working on #36075.
---
The SQLInsertCompiler.execute_sql changes (7254f1138d9c51fa558229c39c9559b369c4278a) made to accommodate returning_fields support (#29444) failed to account for the possibility that backends not supporting returning columns from inserts (MySQL, SQLite < 3.35, and Oracle when the use_returning_into option is disabled) could still request returning_fields and not have an auto-primary key when GeneratedField was introduced.
Given the following model
class TestModel(models.Model): id = models.UUIDField(primary_key=True, default=uuid.uuid4) a = models.IntegerField() b = models.GeneratedField( expression=F("a"), output_field=models.IntegerField(), db_persist=True, )
Attempting to perform TestModel.objects.create(a=1) crashes because even if there is no usage of AutoField we still attempt to call last_insert_id and for the UUIDField primary key (which will either return 0 or crash on Oracle since it attempts to retrieve a non-existing sequence) and then try to convert 0 to a uuid.UUID which crashes with
Traceback (most recent call last):
File "/django/source/tests/model_fields/test_generatedfield.py", line 361, in test_create_with_non_auto_pk
obj = GeneratedModelNonAutoPk.objects.create(a=1, b=2)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/django/source/django/db/models/manager.py", line 87, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/django/source/django/db/models/query.py", line 663, in create
obj.save(force_insert=True, using=self.db)
File "/django/source/django/db/models/base.py", line 901, in save
self.save_base(
File "/django/source/django/db/models/base.py", line 1007, in save_base
updated = self._save_table(
^^^^^^^^^^^^^^^^^
File "/django/source/django/db/models/base.py", line 1168, in _save_table
results = self._do_insert(
^^^^^^^^^^^^^^^^
File "/django/source/django/db/models/base.py", line 1209, in _do_insert
return manager._insert(
^^^^^^^^^^^^^^^^
File "/django/source/django/db/models/manager.py", line 87, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/django/source/django/db/models/query.py", line 1845, in _insert
return query.get_compiler(using=using).execute_sql(returning_fields)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/django/source/django/db/models/sql/compiler.py", line 1927, in execute_sql
return list(rows)
^^^^^^^^^^
File "/django/source/django/db/models/sql/compiler.py", line 1541, in apply_converters
value = converter(value, expression, connection)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/django/source/django/db/backends/mysql/operations.py", line 327, in convert_uuidfield_value
value = uuid.UUID(value)
^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/uuid.py", line 175, in __init__
hex = hex.replace('urn:', '').replace('uuid:', '')
^^^^^^^^^^^
AttributeError: 'int' object has no attribute 'replace'
Note that in a case of a non-integer primary, like described above we get a crash, but in a case of an integer one the issue is even more insidious as the bogus value (which will be either be 0 or whatever value the last inserted table with an auto primary key was) will happily pass the conversion layer and then be wrongly assigned to GeneratedField value. In other words, if the model is defined like the following instead
class TestModel(models.Model): id = models.IntegerField(primary_key=True) a = models.IntegerField() b = models.GeneratedField( expression=F("a"), output_field=models.IntegerField(), db_persist=True, )
Then we'll encounter the following problem
>>> obj = TestModel.object.create(id=1, a=42) >>> obj.b 1 # This should be 42!!
Change History (5)
comment:1 by , 10 months ago
| Has patch: | set |
|---|
comment:2 by , 10 months ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 10 months ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
comment:4 by , 10 months ago
| Patch needs improvement: | unset |
|---|
comment:5 by , 10 months ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
In 9e55201: