Model.save() refactoring incompatibilities
|Reported by:||akaariai||Owned by:||nobody|
|Component:||Database layer (models, ORM)||Version:||master|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Previously when saving an object with primary key set, Django first did a SELECT to check if an row existed in the database. If it existed, an UPDATE was performed, otherwise an INSERT was performed. In Django 1.6 this is change so that a direct UPDATE is done. The database should return how many rows were found for the UPDATE. If no rows were found, then Django knows to perform an INSERT. If rows were found, then the UPDATE is already done. This way reduces one query in the common case where there is a matching row in the database.
However, there are some rare cases where the database doesn't return "row found" for UPDATE even if there is a matching row in the DB. One case is PostgreSQL and ON UPDATE trigger that returns NULL (returning NULL from ON UPDATE trigger skips the UPDATE operation). There are likely some other cases, at least when considering all the 3rd party backends.
When the database doesn't return "row found" for UPDATE, the result is that Django tries to insert and an IntegrityError follows. Thus, those users who are hit by the above condition simply cannot update existing objects. Django itself shouldn't ever generate schemas that have this problem, but again, who knows about third party backends.
It would be really nice to offer some fallback way for users who have the above problem. One possibility is introduced in https://github.com/akaariai/django/commit/1ca398b0352b11868ec236f92cf17b6ce82ff88c, that is Model.Meta option select_on_save that switches the model back to the old behaviour.
I think at least adding the "forced_update" argument to Model._do_update() before release candidate would be good. If it turns out later on that we want "select_on_save" flag, then there is no need to change the _do_update() signature mid stable release. Even if it is private API that would be nice for users. In addition, this allows those who have this problem to override _do_update(), and avoid the problem (private API override, but still better than "use raw SQL").
In addition release notes should be updated, the current note about this is too small, and isn't easy to understand if you don't already know what it means (same thing goes for .save() documentation). This can be done post RC1.
It is worth considering if adding the "select_on_save" flag should be done. A possibility is to wait-and-see, and add in mid 1.6 if enough complaints are risen.
I am marking this as release blocker, RC1 for the _do_update() signature change, and 1.6 release for the release notes change.
Change History (5)
comment:3 Changed 2 years ago by Anssi Kääriäinen <akaariai@…>
- Resolution set to fixed
- Status changed from new to closed