Model._do_update isn't as useful as it could be
|Reported by:||gavinwahl@…||Owned by:||nobody|
|Component:||Database layer (models, ORM)||Version:||master|
|Cc:||Triage Stage:||Ready for checkin|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Ticket #16649 refactored model saving and added a _do_update method that does the actual database UPDATE query. However, a similar database query that checks that a row with the pk still exists under certain conditions is still in _save_base, and as such can't be overridden by a subclass.
_save_base shortcuts _do_update when (from the comment):
We can end up here when saving a model in inheritance chain where update_fields doesn't target any field in current model. In that case we just say the update succeeded. Another case ending up here is a model with just PK - in that case check that the PK still exists.
I am using _do_update to implement optimistic concurrency control in django-optimistic-lock. In order to implement it correctly for inherited models, I need to add my own condition to the EXISTS query that _save_base does. If this query were pushed down to _do_update, I would be able to do this. Otherwise, I can't add my logic because it is deep inside the _save_base code.
I have a failing test in my library that demonstrates: https://github.com/gavinwahl/django-optimistic-lock/blob/4472b73b3eeb7ce2ea19164c75d70aa39249aa34/tests/tests/tests.py#L72
I modified Django slightly to move the existence query into _do_update, and I'm able to implement the behavior I need for my library. See https://github.com/gavinwahl/django-optimistic-lock/commit/90be77cb356a7b65410df3f52407cc38da2f8e22.
Change History (5)
comment:1 Changed 23 months ago by akaariai
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Accepted