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