id,summary,reporter,owner,description,type,status,component,version,severity,resolution,keywords,cc,stage,has_patch,needs_docs,needs_tests,needs_better_patch,easy,ui_ux 22967,Model._do_update not always consistent with the precise meaning of returned value,Jozef,Jozef,"''This is a bit obscure corner-case bug which might seem unimportant, but the solution is easy and actually only makes the code simpler (removes one nested if) and more consistent.'' When {{{select_on_save}}} is set, {{{Model._do_update()}}} ignores the result of {{{Model._update()}}} if it learnt that the object exists '''in a previous DB call'''. That last thing is crucial, as the object could have already been deleted by concurrent worker just in-between these two ORM calls ({{{exists}}} and {{{_update}}}). In such case, the {{{_update()}}} call will fail, resulting in nothing being actually updated. However, {{{_do_update()}}} method will currently return '''True''' regardless of {{{_update()}}} result, which is inconsistent with the common (and expected) behavior, and also in contrast with the method's docs (saying: ''If the model was updated (in the sense that an update query was done and a matching row was found from the DB) the method will return True.'') As a consequence, object will not be saved into the database, because {{{_save_table()}}} won't call {{{_do_insert()}}}, being misled by {{{_do_update()}}}. Now you may oppose that this does not really matter that much, since it has been caused by concurrent modifications of the same data, which is considered non-deterministic anyway (so noone has the right to complain that the object is not there at the end). However, here's one example that vouches for this bug to be fixed: if you happen to have a trigger in your DB counting the number of times an object was changed (updated/inserted), you'd be missing one (which is no longer justifiable by concurrent save() and delete() operations). As it sometimes happens, it is IMHO impossible to create a unit test for this bug, as it is too low level thing, exploitable only in a concurrent environment, and with some (bad) luck. I only found the bug when studying the code. So that's why I tried to provide a solid explanation, believing the fix I'm about to make should be obvious and mergable without worries.",Bug,closed,"Database layer (models, ORM)",dev,Normal,fixed,select_on_save concurrency race-condition,Jozef,Ready for checkin,1,0,0,0,0,0