Model._do_update isn't as useful as it could be

[gavinwahl@…]

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.

My patch: ​https://github.com/gavinwahl/django/commit/cfebca2f11655b64b21cab554371c2696d4faa36/

[Anssi Kääriäinen]

_do_update() is private API but making private API easier to use by third party apps is a good idea. So, seems like a good change to me.

[gavinwahl@…]

I opened a pull request: ​https://github.com/django/django/pull/1018

[Tim Graham <timgraham@…>]

In 616f3c4a795ba6d74c5f28d200d1921e998232f4:

Fixed #20272 - Moved update_fields existence check into Model._do_update.

Thanks Gavin Wahl.