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. 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. 

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. 

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. 

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. 

As far as I can see these four lines have been in base.py since Anssi's 
refactoring. 

kind regards,
Jeroen

 

> > But since I can easily recover from this exception, I think it would be 
> > nice if these exceptions would be typed in a more specific subclass so 
> > that I can catch them and do something nice for the end-user. 
>
> I think it would be reasonable to raise a sub-type of DatabaseError for 
> these "nothing was updated" errors, so they can be more easily caught 
> and handled without parsing exception messages. Could you file a ticket 
> (and pull request?) for that? 
>
> > As a side note, you might want to check that a single row is updated, 
> > and not "more than zero"; just in case a primary key is composed from 
> > more than one column and there's some other error... 
>
> Django models don't support compound primary keys at this point (though 
> there is hope that they may in the future). If you can show a situation 
> that is achievable through public APIs where the check for "more than 
> zero" gives wrong results, that would be worth filing a separate bug for. 
>
> 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/cee4f3f9-3f33-4362-9c9d-024b70691f5b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to