Description

The bug is that when running outside transaction management, Django does a save() after every update/insert. In multitable inheritance, it is possible (although rare) that this will result in half-saved objects. For example, using postgresql, and model inheritance I get these queries:

Ok, here is a patch. In my opinion it does make it easier to understand what is going on in the saving process. I have tried to make sure saving works just like before. The only behavioral change should be committing only once, after all tables have been saved. At least the tests agree I haven't broken anything.

There is one thing I don't like, and that is the parameter name "topmost_concrete". It is needed so that we know to send signals only once, after the last concrete (non-proxy) model's table is saved. Better names (or better logic) welcome. Before this was done using origin, which was self.__class__, until first concrete model was seen, and after that it was None. The information in origin was not needed for anything else than checking when to send signals.

I realized there is one more plain old bug hidden in there: force_insert & force_update are not passed down the inheritance chain. There might be some reason for not doing that for multitable inheritance (does it mean force_insert all the tables, or force_insert just the topmost table?). But for proxy models this is a clear bug.

The reason for the additional method is that the first save_base call is special. The additional method makes it explicit. It is easy to see that we only send the signals for the topmost model. I could not easily see that in the current trunk version. Patch attached. Me likes this version, but maybe this is getting overly cute...

In the latest patch, actual behavioral changes are:

commit_unless_managed is called only once, after all tables have been saved.

force_update & force_insert do have an effect for proxy models. As before, these are not passed down to multitable inheritance chains.

These changes have tests in the patch. All tests passed on SQLite.

The rest is just cleanup. There should not be any changes in how the per-table saving is done (the large if not meta.proxy: branch in trunk code is just reindented).

I wonder if I am once again extending the scope of the ticket too much. Maybe rename this to cleanup of model.save_base()? If not, just say and I will create a separate ticket for the general cleanup and leave this ticket just for the transaction case. In that case, the first submitted patch is the way to go.

Last: is it intentional that force_insert & force_update are not passed down the inheritance chain in multitable inheritance case?