Opened 5 years ago
Closed 5 years ago
#31321 closed Cleanup/optimization (wontfix)
Unexpected behavior of `update_or_create`, misleading flag `created`.
Reported by: | plushchaynikolay | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.0 |
Severity: | Normal | Keywords: | get_or_create update_or_create created primary_key |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
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 doesobj.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
intofound
- 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
more, desu
Change History (1)
comment:1 by , 5 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Summary: | Unexpected behavior of `update_or_create`, misleading flag `created` → Unexpected behavior of `update_or_create`, misleading flag `created`. |
get_or_create()
doesn't override existing objects so it's not affected by described behavior.About
update_or_create()
, it's documented that this is as a shortcut to boilerplatish a concrete code, it's also documented how Django behaves when you updatepk
withNone
. 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 usedmodel_to_dict()
instead of passing dictionary directly todefaults
.IMO documenting this edge case or changing
created
tofound
would be even more misleading. Changing behavior forAutoField
's would be backward incompatible and I don't see a reason to do this.