This project is archived and is in readonly mode.

'increment' and 'decrement' for ActiveRecord::Relation

#5614

Hi guys,

I've added 2 new methods to ActiveRecord::Relation

http://github.com/pch/rails/commit/b9bcb52966addb714f5d634fe78f0899...
http://github.com/pch/rails/commit/486b583cc753fc53dc9a979f3d55c58b...

Essentially, this allows for executing queries like:

UPDATE some_table SET comment_count = comment_count + 1 WHERE id = 6;

Article.create(:title => "Hello", :counter => 0)
Article.increment(1, :counter)

ActiveRecord::Persistence#increment and #decrement need to fetch values from the database before updating them. That way, in some cases, there's a possibility of desynchronizing field value by updating it after it had been already overwritten in another request:

# counter == 0
# request 1:
article = Article.find(1)

# request 2:
article = Article.find(2)

#request 1:
article.increment(:counter)
article.save # counter == 1

#request 2:
article.increment(:counter)
article.save # counter == 1

Let me know what you think about it.

Reported by ronin-22245 (at lighthouseapp) · September 11th, 2010 @ 08:38 AM

Activity

ronin-22245 (at lighthouseapp)
- Tag set to decrement, increment
September 11th, 2010 @ 08:39 AM
Mikel Lindsaar
- Milestone cleared.
- Assigned user set to Mikel Lindsaar
- Importance changed from to Low
September 11th, 2010 @ 11:50 AM
Jeff Kreeftmeijer
+1

This seems like a nice addition and it'll probably keep people from executing two queries to achieve something like this. :)

September 11th, 2010 @ 02:24 PM
Grzegorz Kazulak
+1 on this.

September 11th, 2010 @ 05:50 PM
Jeff Kreeftmeijer
- Tag changed from decrement, increment to activerecord, patch, relation
October 10th, 2010 @ 09:22 AM
Aditya Sanghi
+1 but i think the API could be improved a bit.

How about a default column to increment/decrement, instead of being an mandatory param?

October 11th, 2010 @ 11:11 AM
Jeremy Kemper
- Milestone set to 3.0.2
[bulk edit]

October 15th, 2010 @ 11:02 PM
Ryan Bigg
- State changed from new to verified
Please attach a patch (as a file) to this ticket.

October 16th, 2010 @ 02:03 AM
Ryan Bigg
- State changed from verified to open
October 16th, 2010 @ 02:03 AM
Ryan Bigg
- Tag changed from sheepskin boots, activerecord, patch, relation to activerecord patch relation
Automatic cleanup of spam.

October 16th, 2010 @ 02:23 AM
ronin-22245 (at lighthouseapp)
Attaching the patch

October 16th, 2010 @ 11:30 AM
Andrea Campi
- Tag changed from activerecord patch relation to activerecord relation, patch
I like the idea and the implementation, but I can't +1 until there's some documentation :)
In particular, I think the documentation from these methods should discuss the difference from @ActiveRecord::Persistence#{increment,decrement}@, and the documentation for those methods should be modified to mention these new ones.
Other than that, looks great.

October 16th, 2010 @ 08:25 PM
ronin-22245 (at lighthouseapp)
You're right Andrea. I've updated the patch with some documentation. Let me know what you think.
- activerecord_increment_decrement_patch.diff 8.37 KB
October 17th, 2010 @ 11:39 AM
Andrea Campi
+1 looks and works great

October 17th, 2010 @ 12:06 PM
Łukasz Strzałkowski
I'm for moving attr_name as first parameter. It's more natural and intuitive order for me. Like this:
```
Article.decrement(:counter, 1)
Article.decrement(:counter, [1, 2, 3])
```
What do you think?
October 17th, 2010 @ 05:54 PM
Santiago Pastorino
- Milestone cleared.
[bulk edit]

November 15th, 2010 @ 09:15 PM
ronin-22245 (at lighthouseapp)
- Assigned user changed from Mikel Lindsaar to José Valim
November 17th, 2010 @ 09:30 AM
José Valim
- Assigned user changed from José Valim to Aaron Patterson
Łukasz Strzałkowski, the API should be the same as the one in AR::Base. Assigning to Aaron Patterson, since he's AR's BFF.

November 25th, 2010 @ 09:50 AM
Aaron Patterson
@Łukasz, I agree with José. Please make the api match the api in AR::Base.

Also, I don't think the ArgumentError is worth our time. It's possible there are attributes in the database that AR doesn't know about. But even if AR does know about all of the attributes, the generated SQL will fail and we get an exception raised with no extra code.

December 15th, 2010 @ 09:47 PM
markus
```
  Article.increment_counter(:counter, 1)
  Article.increment_counter(:counter, [1, 2, 3])
```
already exists in ActiveRecord::CounterCache. Why not just mention that in the documentation of ActiveRecord::Persistence#increment and #decrement instead of adding more methods?
January 15th, 2011 @ 06:05 PM
Santiago Pastorino
- Milestone set to 3.0.5
[bulk edit]

February 12th, 2011 @ 09:58 PM
Aaron Patterson
- Milestone changed from 3.0.5 to 3.1
February 25th, 2011 @ 12:45 AM
Aaron Patterson
- State changed from open to invalid
This feature already exists in 3.0.x:

https://github.com/rails/rails/blob/3-0-stable/activerecord/lib/act...

https://github.com/rails/rails/blob/3-0-stable/activerecord/test/ca...

February 25th, 2011 @ 06:50 PM