#36472: GeneratedField(primary_key=True) crashes on create(), and other issues
-------------------------------------+-------------------------------------
Reporter: David Sanders | Type:
| Uncategorized
Status: new | Component: Database
| layer (models, ORM)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Related to: #36466
Given the following model:
{{{
class Foo(Model):
foo = IntegerField()
bar = GeneratedField(
expression=F("foo") + 5,
output_field=IntegerField(),
db_persist=True,
primary_key=True,
)
}}}
Django will crash on the following:
`create()`
{{{
>>> Foo.objects.create(foo=100)
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/path/to/django/db/models/manager.py", line 87, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/query.py", line 665, in create
obj.save(force_insert=True, using=self.db)
File "/path/to/django/db/models/base.py", line 873, in save
self.save_base(
File "/path/to/django/db/models/base.py", line 965, in save_base
updated = self._save_table(
^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/base.py", line 1066, in _save_table
if not self._is_pk_set(meta):
^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/base.py", line 688, in _is_pk_set
pk_val = self._get_pk_val(meta)
^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/base.py", line 677, in _get_pk_val
return getattr(self, meta.pk.attname)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/query_utils.py", line 221, in __get__
val = self._check_parent_chain(instance)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/query_utils.py", line 242, in
_check_parent_chain
return getattr(instance, link_field.attname)
^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'attname'
}}}
if we patch `DeferredAttribute` to account for `None` with
{{{
--- a/django/db/models/query_utils.py
+++ b/django/db/models/query_utils.py
@@ -238,7 +238,7 @@ class DeferredAttribute:
"""
opts = instance._meta
link_field = opts.get_ancestor_link(self.field.model)
- if self.field.primary_key and self.field != link_field:
+ if link_field and self.field.primary_key and self.field !=
link_field:
return getattr(instance, link_field.attname)
return None
}}}
then we get infinite recursion:
{{{
>>> Foo.objects.create(foo=100)
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/path/to/django/db/models/manager.py", line 87, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/query.py", line 665, in create
obj.save(force_insert=True, using=self.db)
File "/path/to/django/db/models/base.py", line 873, in save
self.save_base(
File "/path/to/django/db/models/base.py", line 965, in save_base
updated = self._save_table(
^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/base.py", line 1066, in _save_table
if not self._is_pk_set(meta):
^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/base.py", line 688, in _is_pk_set
pk_val = self._get_pk_val(meta)
^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/base.py", line 677, in _get_pk_val
return getattr(self, meta.pk.attname)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/query_utils.py", line 223, in __get__
if not instance._is_pk_set():
^^^^^^^^^^^^^^^^^^^^^
>>>> etc etc etc <<<
RecursionError: maximum recursion depth exceeded
}}}
Patching `DeferredAttribute` seems to address the infinite recursion and
allow creation (but this will block the update of the generated field's
value):
{{{
--- a/django/db/models/query_utils.py
+++ b/django/db/models/query_utils.py
@@ -219,7 +219,7 @@ class DeferredAttribute:
# Let's see if the field is part of the parent chain. If so
we
# might be able to reuse the already loaded value. Refs
#18343.
val = self._check_parent_chain(instance)
- if val is None:
+ if val is None and not self.field.generated:
if not instance._is_pk_set():
raise AttributeError(
f"Cannot retrieve deferred field {field_name!r} "
}}}
Then there are issues around fetching, eg consider the following:
{{{
>>> foo = Foo.objects.get(...)
>>> foo.foo = <some other value>
>>> foo.save()
>>> foo.refresh_from_db()
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/path/to/django/db/models/base.py", line 757, in refresh_from_db
db_instance = db_instance_qs.get()
^^^^^^^^^^^^^^^^^^^^
File "/path/to/django/db/models/query.py", line 635, in get
raise self.model.DoesNotExist(
generated_as_pk.models.Foo.DoesNotExist: Foo matching query does not
exist.
}}}
These are just some basic tests, there may be other issues not yet
noticed.
I don't think it's as bad as banning `primary_key=True` though. Simon has
a branch for #27222 which seems to fix the `refresh_from_db()` issue.
Perhaps the infinite recursion can be solved simply/elegantly as well?
--
Ticket URL: <https://code.djangoproject.com/ticket/36472>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/0107019783aa8663-e8907043-1cdc-4307-8825-77b3def44b55-000000%40eu-central-1.amazonses.com.
