#5614 'increment' and 'decrement' for ActiveRecord::Relation - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#5614 ✓invalid

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

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

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.

Comments and changes to this ticket

You flagged this item as spam.
ronin-22245 (at lighthouseapp) September 11th, 2010 @ 08:39 AM
- Tag set to “decrement, increment”
Mikel Lindsaar September 11th, 2010 @ 11:50 AM
- Milestone cleared.
- Assigned user set to “Mikel Lindsaar”
- Importance changed from “” to “Low”
Jeff Kreeftmeijer September 11th, 2010 @ 02:24 PM
+1

This seems like a nice addition and it'll probably keep people from executing two queries to achieve something like this. :)
You flagged this item as spam.
Grzegorz Kazulak September 11th, 2010 @ 05:50 PM
+1 on this.
Jeff Kreeftmeijer October 10th, 2010 @ 09:22 AM
- Tag changed from “decrement, increment” to “activerecord, patch, relation”
Aditya Sanghi October 11th, 2010 @ 11:11 AM
+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?
Jeremy Kemper October 15th, 2010 @ 11:02 PM
- Milestone set to “3.0.2”
[bulk edit]
Ryan Bigg October 16th, 2010 @ 02:03 AM
- State changed from “new” to “verified”
Please attach a patch (as a file) to this ticket.
Ryan Bigg October 16th, 2010 @ 02:03 AM
- State changed from “verified” to “open”
Ryan Bigg October 16th, 2010 @ 02:23 AM
- Tag changed from “sheepskin boots, activerecord, patch, relation” to “activerecord patch relation”
Automatic cleanup of spam.
You flagged this item as spam.
ronin-22245 (at lighthouseapp) October 16th, 2010 @ 11:30 AM
Attaching the patch
You flagged this item as spam.
Andrea Campi October 16th, 2010 @ 08:25 PM
- 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.
You flagged this item as spam.
ronin-22245 (at lighthouseapp) October 17th, 2010 @ 11:39 AM
You're right Andrea. I've updated the patch with some documentation. Let me know what you think.
- activerecord_increment_decrement_patch.diff 8.4 KB
You flagged this item as spam.
Andrea Campi October 17th, 2010 @ 12:06 PM
+1 looks and works great
You flagged this item as spam.
Łukasz Strzałkowski October 17th, 2010 @ 05:54 PM
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?
Santiago Pastorino November 15th, 2010 @ 09:15 PM
- Milestone cleared.
[bulk edit]
You flagged this item as spam.
ronin-22245 (at lighthouseapp) November 17th, 2010 @ 09:30 AM
- Assigned user changed from “Mikel Lindsaar” to “José Valim”
José Valim November 25th, 2010 @ 09:50 AM
- 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.
Aaron Patterson December 15th, 2010 @ 09:47 PM
@Ł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.
You flagged this item as spam.
markus January 15th, 2011 @ 06:05 PM
```
  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?
Santiago Pastorino February 12th, 2011 @ 09:58 PM
- Milestone set to “3.0.5”
[bulk edit]
Aaron Patterson February 25th, 2011 @ 12:45 AM
- Milestone changed from “3.0.5” to “3.1”
Aaron Patterson February 25th, 2011 @ 06:50 PM
- 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...