Hi Carl, Thanks for taking the time to respond.
I'm still wondering what's best for save_table to do. Pre-1.6 Django code would end up with a new object in my situation, and no complaints. Apparently, that worked, for me. I'm inclined to think that is still what I want for the update-y variants of save(). Or maybe I should consider using manager.update. I've put up a sample of the multiprocessing situation on github: https://github.com/jeroenp/glowing-octo-cyril/blob/master/steps.py kind regards Jeroen On Wednesday, November 19, 2014 4:25:14 AM UTC+1, Carl Meyer wrote: > > Hi Jeroen, > > On 11/18/2014 10:12 AM, Jeroen Pulles wrote: > > On Tuesday, November 18, 2014 5:00:06 PM UTC+1, Carl Meyer wrote: > > > > Hi Jeroen, > > > > On 11/18/2014 08:44 AM, Jeroen Pulles wrote: > > > the model.save() arguments `force_update` and `update_fields` > > cause an > > > explicit check if the update statement matched any rows. > > > > > > Every once in a while one of my applications runs into a "race > > > condition" where one process does an update (with one of the model > > save > > > arguments) and another process just removed the same object. You > then > > > get a very generic DatabaseError with the message "Forced update > > did not > > > affect any rows.", or "Save with update_fields did not affect any > > > rows.". This invariably makes me scratch my head and wonder what > > happened. > > > > > > As per ticket #21761, I don't understand why that is an error and > a > > > regular call to save() doesn't throw an error. > > > > A regular call to save() will do an INSERT if no rows are updated by > an > > UPDATE, so neither case is an error. But if you explicitly say "I > want > > an update" and there is nothing to update, in many cases that is a > bug > > in your code, so we raise an error. > > > > > > I'm not sure you understand my situation: > > > > A regular .save() on an existing object with primary key will cause an > > UPDATE; No checks, no INSERTs following that statement. > > That's not accurate. A .save() on an existing object, with primary key > set, will cause an INSERT if no rows are updated (that is, its primary > key does not exist in the database.) You can easily verify this by > trying it out. > > This is why there is no error - you haven't explicitly asked for an > UPDATE or an INSERT, so Django will happily decide which to do based on > whether the row exists or not. Neither condition is unexpected or an > error. > > > My "race > > condition" where another process DELETEd the object causes the same "0 > > matching rows" return value from the underlying update call, but with > > .save() there are no exceptions. > > Correct, because an INSERT takes place instead. > > > If I were to change my calling code from .save(update_fields=['foobar']) > > to .save() I would be a happy bunny. No exception, yay! I'll have to > > deal with some mayhem from no longer having the record, but I can deal > > with that. > > You would have the record, because it would be re-INSERTed. > > > So I'm still confused why the explicit arguments would be somehow > > different. Why would Django suddenly care about a bug in the caller's > > code--I have loads ;-) --? The database didn't complain, at least. > > By calling .save() without arguments, you are telling Django "merge this > object: UPDATE if it already exists, INSERT it otherwise." Neither > condition is an error, both are within the expected range of > possibilities. > > By using `force_update` or `update_fields`, you are telling Django "I > expect this row to already exist, and I want you to update it." Thus, > failure to update it is an error condition, because Django was unable to > do the thing that you asked it to do. > > > My personal preference would be to remove the four lines with the > > exception raising. I can't imaging anyone is actively expecting these > > (generic) exceptions in their calling code. If you do get the exception, > > my position is that it is highly unlikely that the caller made a > > programming error. > > I understand that you have a case where you don't consider it a > programming error (though one could argue that you should be using a > transaction and select-for-update to prevent this race condition), but I > don't think either of us has real data on that question. There are > certainly plausible programming errors that would lead to this; one > example was already given in this thread (using `update_fields` or > `force_update` on a new, never-saved instance). > > As I said before, I think making these errors a more specific sub-type > to make them easier to catch would be reasonable. But I don't think they > should be removed. They represent a case where Django was a) unable to > do what you asked it to do, and b) as a result Django is throwing away > your model data unsaved. That's something that I don't think should pass > silently. > > Carl > > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To post to this group, send email to django-developers@googlegroups.com. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/473cfdc5-0b56-4427-b676-f487433a114f%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
