account = Account.find(1) account.balance += 1000 account.save
Peace, Phillip
account = Account.find(1) account.balance += 1000 account.save
Peace, Phillip
Phillip Koebbe wrote:
Hi. Maybe it's too early or maybe I'm just dense, but would you mind explaining your statement? Maybe I don't know enough about your specific situation, but what I offered is the standard Rails way of updating the attributes of an object. What are the business rules around the update of this "account"?
Thanks, Phillip
Hm. For some reason, part of my reply was lost as it was applied to ruby-forum ( I participate through email). Let me restate then:
Why do you say this is a source of race conditions? Are you using this in a situation where many people could be updating the record at the same time? If only one person could be updating, how can it be a potential race? If this is in a multi-user environment, you didn't mention that earlier, so how was I to know?
Peace, Phillip
Phillip Koebbe wrote the following on 29.12.2007 13:51 :
account = Account.find(1) account.balance += 1000 account.save
That's a source of race conditions.
Account.update_all({ :id => 1 }, 'balance = balance + 1000') is safe under any condition (at least with PostgreSQL).
Hi. Maybe it's too early or maybe I'm just dense, but would you mind
explaining your statement? Maybe I don't know enough about your
specific situation, but what I offered is the standard Rails way of
updating the attributes of an object. What are the business rules
around the update of this "account"? I don't know the business rules of this particular case, but usually simultaneous account balance manipulations are expected to behave in the way they would have if they'd been serialized. So clearly you'd want to avoid the race condition I'm referring to in my previous post.
I merely pointed out that your example can't enforce this by itself an presented a solution. In your case you could use a transaction and lock the account to be sure using account.lock or Account.find(1, :lock => true), it may makes sense in some situations but is slower and probably more error-prone than the update_all solution.
Lionel
Phillip Koebbe wrote the following on 29.12.2007 13:59 :
Why do you say this is a source of race conditions? Are you using
this in a situation where many people could be updating the record at
the same time? If only one person could be updating, how can it be a
potential race? If this is in a multi-user environment, you didn't
mention that earlier, so how was I to know? I'm not the original poster so I'm not using this code. I was simply pointing a potential source of problems that could potentially be of concerns to people using the kind of code you presented.
Lionel
Ah, so you're not. Okay, then it is too early. Maybe I should drink more coffee and do some jumping jacks or something before I get post happy.
Thanks for clearing that up, Lionel. You're absolutely right. Used in the wrong situation, the code that I provided would not be reliable. Used in the right situation, however, it is perfectly acceptable.
Peace, Phillip
Not that I'm suggesting it, but Account.update_counters(1, :balance => 1000) would also work.
///ark
Mark Wilden wrote:
Account.update_all({ :id => 1 }, 'balance = balance + 1000') Not that I'm suggesting it,
Why? Seems pretty safe and concise to me
but Account.update_counters(1, :balance => 1000) would also work.
I'll have to remember this when switching to Rails 2 (I looked it up in 1.2.6's rdoc and didn't find it).
BTW, I think I was confused by the initial examples, the syntax in my example is not right, it should probably be:
Account.update_all('balance = balance + 1000', { :id => 1 })
and what I know works on Rails since 1.0 is:
Account.update_all('balance = balance + 1000', 'id = 1')
Lionel
Just a casual glance... Would like this work?
module ActiveRecord class Base def change_value(attribute, amount) self[attribute] += amount end
def change_value!(attribute, amount) change_value(attribute, amount).update_attribute(attribute, self[attribute]) end
def increment(attribute, amount = 1) change_value(attribute, amount) end
def increment!(attribute, amount = 1) change_value(attribute, amount) end
def decrement(attribute, amount = 1) change_value(attribute, -amount) end
def decrement!(attribute, amount = 1) change_value(attribute, -amount) end end end
Well, if you really like it, it’s ticket 10656 (http://dev.rubyonrails.org/attachment/ticket/10656/change_ar_increment_to_accept_by_argument.diff)). +1 it if you think it’s a good idea.
Happy New Year (early),
–steve
I think the diff reads that way. Note that the method signature changed so the first def is in red (deleted), then the comment(s), then the new def with the new method signature are in green (added). Did I miss something?
This looks good (some of the calls to change_value should be calling change_value!), but I think it's too close to existing functionality (update_counters).
///ark
module ActiveRecord class Base def change_value(attribute, amount) self[attribute] += amount end
This looks good (some of the calls to change_value should be calling change_value!), but I think it's too close to existing functionality (update_counters).
Actually, there are two patches in for this. The tickets are:
10542 (not mine)
and
10656 (mine)
The primary difference is syntactic, but 10542 is already verified, and thus may be the patch that is accepted. I've asked for a discussion about the syntax on <http://dev.rubyonrails.org/ticket/10542#comment:7 >. The other patch allows for syntax like:
@player.increment! :score, 3
My suggested patch allows for a slightly different syntax:
@player.increment! :score, :by => 3
it also opens the door to future enhancements like:
@player.increment! :score, :by => 3, limit => 100, :if => :positive
I'm not sure there's a use case for anything too complex in increment and decrement, though