This project is archived and is in readonly mode.

#228 ✓wontfix
Damian Janowski

Add add_counter_cache_column to ActiveRecord migrations

Reported by Damian Janowski | July 17th, 2008 @ 01:52 AM

This method makes it straightforward to add new columns to work with :counter_cache on ActiveRecord models. Apart from adding the column (default 0), it will update the counters for all the records.

For example:

def self.up
  add_counter_cache_column :posts, :comments
end

Creates a new comments_count column on posts and updates all current posts with the comments count -- equivalent to:

Post.reset_column_information
Post.find(:all).each do |post|
  Post.update_counters post.id, :comments_count => post.comments.count
end

Updated docs and test included.

Comments and changes to this ticket

  • Ryan Bates

    Ryan Bates May 20th, 2008 @ 10:00 PM

    Nice. Would it fit to have a "t.counter_cache" method for use in create_table and change_table as well?

  • Lawrence Pit

    Lawrence Pit May 20th, 2008 @ 10:52 PM

    +1 for adding t.counter_cache.

    About the loop: if there are millions of rows to be updated then the proposed code is too slow. The counter cache column can be updated with 1 UPDATE statement: UPDATE posts SET comments_count = (SELECT COUNT(*) FROM comments WHERE comment.post_id = post.id)

  • Guillermo Álvarez

    Guillermo Álvarez May 20th, 2008 @ 11:29 PM

    I think it could be cleaner something like these

    t.references :post, :couter=> true

    like we already do with for example null or polymorphic

  • Ryan Bates

    Ryan Bates May 21st, 2008 @ 12:19 AM

    @Guillermo, interesting idea, but then how would one add the cache column if the post_id column already exists in comments table?

    Since caching usually happens after analyzing performance, I think it is better kept as a separate step.

  • Guillermo Álvarez

    Guillermo Álvarez May 21st, 2008 @ 07:44 AM

    @Ryan_Bates with something like these

    change_table :users do |u|

    u.add_couter :posts

    end

    So you don't have to learn more methods name, because we will use the same as we already have (Well, t.add_counter is not already implemented). I think its more logical at these manner.

  • Damian Janowski

    Damian Janowski May 21st, 2008 @ 05:42 PM

    @Lawrence Pit: that's a good idea, but it doesn't work if I have non-standard associations (different foreign keys or table names). I guess edge cases would have to be done by hand. However, I'll update the patch to do :select => klass.primary_key for some performance optimization.

  • Damian Janowski

    Damian Janowski May 21st, 2008 @ 05:53 PM

    @Ryan Bates: adding the method to create_table and change_table is not so simple. Those take a TableDefinition, which is just that, a list of columns to be created after the block is called. So we'd need to implement a simple callback mechanism in order to be able to run the code that populates the counters...

  • josh

    josh July 17th, 2008 @ 01:54 AM

    • State changed from “new” to “open”
    • Assigned user set to “josh”
    • Tag set to activerecord, migrations
  • Damian Janowski
  • josh

    josh July 19th, 2008 @ 01:42 AM

    After scanning over this ticket, I thought it was totally unnecessary at first. However, I really like the update counters migration that brings your counters update to date (nice touch).

    Like Ryan said, you usually think about this stuff later so "t.counter_cache" isn't really needed.

    Is this the final API you've decided on?

    add_counter_cache_column :posts, :comments
    
  • josh

    josh July 19th, 2008 @ 06:47 PM

    Talked with some other peeps. Not really in favor of yet another migration helper.

    However, I still like the cache resync helper. Maybe

    # Reload comment counter cache
    post.comments.count(reload = true)
    
  • Ryan Bates

    Ryan Bates July 21st, 2008 @ 11:53 PM

    • Tag changed from activerecord, migrations to activerecord, migrations, patch

    I like the look of the reload counter cache option, but I wonder if "count" is the right method for this. To me it makes more sense to use "size" since that is what normally uses the cache column. You could pass true to force the reload.

    post.comments.size(true)
    

    This way it parallels passing true to associations to reload them.

    Maybe this deserves its own ticket?

  • Pratik

    Pratik July 22nd, 2008 @ 01:19 AM

    • State changed from “open” to “wontfix”

    Yeah. I think it makes sense to create a new ticket. I'd personally prefer something like post.comments.reset_counter_cache I think.

  • Trevor Turk

    Trevor Turk October 14th, 2008 @ 07:41 PM

    Sorry, I see a discussion here:

    http://groups.google.com/group/r...

    ...but I can't find a new ticket about this issue. I've been bitten by this one before - it's very annoying!

  • Trevor Turk
  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 10:14 PM

    • Tag cleared.
    • Importance changed from “” to “Low”

    Automatic cleanup of spam.

  • Ryan Bigg

    Ryan Bigg October 11th, 2010 @ 12:11 PM

    Automatic cleanup of spam.

  • klkk

    klkk May 23rd, 2011 @ 02:53 AM

    louisvuittonwarehouse.com

  • klkk

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages