Add savepoint protection to Model.save()
|Reported by:||Richard Davies <richard.davies@…>||Owned by:||nobody|
|Component:||Database layer (models, ORM)||Version:||master|
|Cc:||richard.davies@…||Triage Stage:||Design decision needed|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
IntegrityError can be raised in save() in several cases:
- because of a conflict on a unique field (example below)
- when using force_insert or force_update if this is not possible
- if there is a race between when save() decides whether to insert/update and when it actually does so
At present, the user should wrap calls to save() with savepoints if they are likely to fail for any of these reasons.
This patch changes save() to always wrap the call to save_base() in savepoints, meaning that all of the above cases leave a working database connection and the user does not need to wrap. It means that "user" code wrapping save() can be removed from several places in Django and its test suite. The complete test suite still passes on Postgresql.
Possibly the savepoints should move _inside_ save_base(), so that direct calls also benefit from them. Doing this naively leads to extra test suite failures, which I have not investigated.
On performance: we are now making many more savepoint calls, which could have performance implications. These calls are no-ops on non-Postgresql backends, so should have little impact there. On Postgresql, time to run the complete test suite was unchanged on my development machine. So, I conclude that there are no performance implications.
Example of IntegrityError from unique field conflict:
I have a simple model: ================ class Test(models.Model): data1 = models.IntegerField(unique=True) data2 = models.IntegerField() ================ and a view using it: ================ def render(request): a = Test() a.data1 = 1 a.data2 = 10 a.save() b = Test() b.data1 = 1 b.data2 = 20 try: b.save() except IntegrityError: # Expected since data1 not unique # savepoint protection needed here for postgresql today pass c = Test() c.data1 = 2 c.data2 = 30 # Today: c is saved by most database backends, but not by postgresql. # savepoint protection would be needed above for postgresql. # # With patch: c is saved by all database backends. c.save() return HttpResponse('Test')
Change History (8)
Changed 5 years ago by Richard Davies <richard.davies@…>
comment:1 Changed 5 years ago by mtredinnick
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Design decision needed