[Django] #36086: 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

[Django]

#36086: 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
-------------------------------------+-------------------------------------
Reporter: Simon | Owner: Simon Charette
Charette |
Type: Bug | Status: assigned
Component: Database | Version: 5.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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

{{{#!python
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`
[https://github.com/django/django/blob/8bee7fa45cd7bfe70b68784314e994e2d193fd70/django/db/models/sql/compiler.py#L1926-L1937
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

{{{#!python
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

{{{#!python
>>> obj = TestModel.object.create(id=1, a=42)
>>> obj.b
1 # This should be 42!!
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36086>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36086: 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
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Simon

| Charette
Type: Bug | Status: assigned

Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Antoliny):

* Attachment "tabular_inline_link.png" added.

[Django]

#36086: 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
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Antoliny):

* Attachment "tabular_inline_link.png" removed.

[Django]

#36086: 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
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36086#comment:1>

[Django]

#36086: 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
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | 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 Mariusz Felisiak):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/36086#comment:2>

[Django]

#36086: 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
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36086#comment:3>

[Django]

#36086: 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
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36086#comment:4>

[Django]

#36086: 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
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
| Charette

Type: Bug | Status: closed

Component: Database layer | Version: 5.0
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"9e552015556661d183a999078a9e846200ef6765" 9e55201]:
{{{#!CommitTicketReference repository=""
revision="9e552015556661d183a999078a9e846200ef6765"
Fixed #36086 -- Fixed crash when using GeneratedField with non-AutoField
pk.

The previous logic was systematically attempting to retrieve
last_insert_id
even for models without an AutoField primary key when they had a
GeneratedField
on backends that can't return columns from INSERT.

The issue affected MySQL, SQLite < 3.35, and Oracle when the
use_returning_into
option was disabled and could result in either crashes when the non-auto
primary key wasn't an IntegerField subclass or silent misassignment of
bogus
insert ids (0 or the previous auto primary key insert value) to the first
defined generated field value.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36086#comment:5>