Help With Update method.

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? :slight_smile:

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? :slight_smile:
  
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 :slight_smile:

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 :slight_smile: