[Django] #31321: Unexpected behavior of `update_or_create`, misleading flag `created`
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
plushchaynikolay |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: 3.0
layer (models, ORM) | Keywords: get_or_create
Severity: Normal | update_or_create created
Triage Stage: | primary_key
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
Unexpected behavior of `update_or_create` (and `get_or_create`) when
accidentally overrode primary_key with None-value.
It was primarily unexpected because of misleading naming of flag
`created`.
{{{
. . .
self.assertEqual(MyModel.objects.count(), 1)
# ok
_, created = MyModel.objects.update_or_create(
somefield=obj.somefield,
defaults={**model_to_dict(obj)}
)
self.assertFalse(created)
# ok
self.assertEqual(MyModel.objects.count(), 1)
# AssertionError: 2 != 1
}}}
`created == False` but `MyModel.objects.count() == 2`, and new object was
actually created.
{{{
>>> model_to_dict(obj)
{'id': None, 'somefield': 'once told me'}
}}}
The field `id` (primary key) is overridden with `None` from
`defaults={**model_to_dict(obj)}`, and when it does`obj.save()`, then
`obj.id` is auto incremented with new value. (Same with `get_or_create`)
We understand that the issue took it's place because of our unknowing of
how this functions work inside. But actually we shouldn't know it.
Also have some suggestions, if You don't mind:
- rename `created` into `found`
- carefully explain this issue in documentation
- check if field is autoincrementable and overridden to a None-value
and ignore overriding
- check if field is autoincrementable and overridden to a None-value
and make a warning
[https://github.com/plushchaynikolay/dj-bug more], desu
--
Ticket URL: <https://code.djangoproject.com/ticket/31321>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: plushchaynikolay | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: get_or_create | Triage Stage:
update_or_create created | Unreviewed
primary_key |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => wontfix
Comment:
`get_or_create()` doesn't override existing objects so it's not affected
by described behavior.
About `update_or_create()`, it's
[https://docs.djangoproject.com/en/3.0/ref/models/querysets/#update-or-
create documented] that this is as a shortcut to boilerplatish a concrete
code, it's also [https://docs.djangoproject.com/en/3.0/topics/db/queries
/#copying-model-instances documented] how Django behaves when you update
`pk` with `None`. If you will take these two into account then this
behavior is expected. I'm not sure why you created a new model instance
and used `model_to_dict()` instead of passing dictionary directly to
`defaults`.
IMO documenting this edge case or changing `created` to `found` would be
even more misleading. Changing behavior for `AutoField`'s would be
backward incompatible and I don't see a reason to do this.
--
Ticket URL: <https://code.djangoproject.com/ticket/31321#comment:1>