Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

[Patch] has_many :through doesn't update counter_cache on join model correctly.

#2824

If you have a counter_cache on a join model used in a has_many :through association, when removing items from the association, the counter_cache doesn't get decremented correctly. e.g.

class Post < ActiveRecord::Base
  has_many :taggings
  has_many :tags, :through => :taggings
end

class Tagging < ActiveRecord::Base
  belongs_to :post, :counter_cache => true
  belongs_to :tag
end

class Tag < ActiveRecord::Base
  has_many :taggings
  has_many :posts, :through => :taggings
end

post = Post.create!()
tag1 = Tag.create!()
tag2 = Tag.create!()
post.tags << tag1
post.reload.taggings_count # => 1
post.tags = [tag2]
post.reload.taggings_count # => 2 !  Should be 1

This happens because #delete_records in HasManyThroughAssociation does a delete_all. The attached patch changes this to a destroy_all which fixes the issue.

Reported by Alex Tomlins · June 22nd, 2009 @ 05:09 PM

State: resolved
Milestone: 3.x
Assigned to: Yehuda Katz (wycats) Yehuda Katz (wycats)
Importance: none

Activity

  1. misza222
    misza222

    Unfortunately this patch does not apply cleanly for me.
    It comes up with an error message

    Applying Make has_many :through update counter_cache on join model correctly.
    error: patch failed: activerecord/test/cases/associations/has_many_through_associations_test.rb:81
    error: activerecord/test/cases/associations/has_many_through_associations_test.rb: patch does not apply
    Patch failed at 0001.
    When you have resolved this problem run "git-am --resolved".
    If you would prefer to skip this patch, instead run "git-am --skip".
    

    June 23rd, 2009 @ 07:27 PM

  2. Alex Tomlins
    Alex Tomlins

    Sorry, I didn't made it clear; that patch is against the 2-3-stable branch.

    June 24th, 2009 @ 09:49 AM

  3. misza222
    misza222

    That works fine and updates taggings_count as expected.

    This problem exists in edge rails as well. Is this change going to be somehow merged back there or patch needs to be created to deal with this issue in edge rails?

    Don't be sorry Alex. It is me who didn't notice 2-3-stable in the tags for this ticket but I am very new to lighthouse and as a matter of fact to rails itself as well (and git :).

    June 25th, 2009 @ 02:01 PM

  4. Yehuda Katz (wycats)
    Yehuda Katz (wycats)
    • Assigned user set to Yehuda Katz (wycats)
    • State changed from new to open

    Would you mind making a patch against edge? I'd like to apply this :)

    July 3rd, 2009 @ 06:47 AM

  5. Alex Tomlins
  6. Peter Haza
    Peter Haza

    Am I the only one not able to download the last patch? I'm getting an XML with some error message.

    September 18th, 2009 @ 07:14 PM

  7. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 3.x

    May 4th, 2010 @ 06:48 PM

  8. Dan Pickett
    Dan Pickett
    • Tag changed from 2-3-stable, bug, counter_cache, has_many_through, patch to 2-3-stable, bug, bugmash, counter_cache, has_many_through, patch

    May 9th, 2010 @ 07:15 PM

  9. mdrozdziel
    mdrozdziel
    • Importance changed from to

    What is the status of this issue? I am using rails 2.3.8 and as far as I can see, this is still the case. Are thre any plans regarding this problem?

    August 19th, 2010 @ 11:49 AM

  10. Neeraj Singh
    Neeraj Singh
    • Tag changed from 2-3-stable, bug, bugmash, counter_cache, has_many_through, patch to rails 3, 2-3-stable, bug, bugmash, counter_cache, has_many_through, patch

    Just tested it on rails edge and it is an issue on edge too.

    August 19th, 2010 @ 02:56 PM

  11. Neeraj Singh
    Neeraj Singh

    @Alex Tomlins: The changes in your patch fixes the problem in rails edge.

    Could you please attach two patches: one for 2-3-stable and another one for rails edge.

    August 19th, 2010 @ 02:59 PM

  12. Alex Tomlins
    Alex Tomlins

    Sure thing.

    I'll try to get onto that this evening (UK time)

    August 19th, 2010 @ 03:34 PM

  13. Alex Tomlins
    Alex Tomlins

    Ok, patches done. They're on github here: http://github.com/unboxed/rails/tree/2-3-counter-cache-fix and here: http://github.com/unboxed/rails/tree/edge-counter-cache-fix

    Let me know if you'd prefer me to create actual patch files for these.

    August 20th, 2010 @ 09:49 AM

  14. Alex Tomlins
    Alex Tomlins
    • Title changed from [PATCH] Clean up "omg" comments to [Patch] has_many :through doesn't update counter_cache on join model correctly.
    • Assigned user set to Yehuda Katz (wycats)
    • Tag changed from 3, documentation, patch, review to rails 3, 2-3-stable, bug, bugmash, counter_cache, has_many_through, patch

    Reverting Spam changes...

    September 2nd, 2010 @ 09:07 AM

  15. Claudio Poli
    Claudio Poli

    I hit the same problem, using rails 2.3.

    I identified another issue, look at this code:

    class PlaylistTrack < ActiveRecord::Base
       belongs_to :playlist, :counter_cache => true, :touch => true
       belongs_to :track
    

    1) adding an object to the collection using <<

    • counter cache works
    • playlist.updated_at fields gets correctly updated

    2) **removing an object from the collection using collection.delete(object)

    • counter cache won't work
    • playlist.updated won't get updated

    In the end there might be additional issues because Playlist might have an observer/callback that runs after_update code.

    In the point 1 this after_update is not triggered by the rails code that updates the counter_cache, but from the :touch one, meaning that adding the :touch options also triggers correctly after_update callbacks.
    Luckily it won't fire twice.

    However in the point 2 since nothing is run also the after_update callback isn't run.

    http://stackoverflow.com/questions/3902995/counter-cache-not-decrem...

    Any plans to integrate this patch please?

    November 17th, 2010 @ 08:14 PM

  16. Claudio Poli
    Claudio Poli

    Patch is absolutely +1 for Rails 2.3, all my tests passes.

    November 17th, 2010 @ 08:18 PM

  17. Ivan Ukhov
    Ivan Ukhov

    The current status of the has_many_through association and its dependent options is the following:

    https://github.com/rails/rails/blob/a3161096c287043c7f22705a771f92d...

            # TODO - add dependent option support
            def delete_records(records)
    

    Also in the reference one can find this line:

    Warning: This option is ignored when used with :through option.

    I've implemented the method, and it works for me prerry well. Could anyone please test the attached patch? It turned out that it's rather hard to run the whole tests of active record...

    Thanks.

    November 20th, 2010 @ 03:49 PM

  18. regis leray
    regis leray

    I try with rails 3.0.4.rc or 3.1.0.beta it s still doesn't work.

    December 29th, 2010 @ 01:20 PM

  19. regis leray
    regis leray

    I apply the path to the branch 3.0-stable (3.0.4rc)

    class User < ActiveRecord::Base
     # need to provide an accessor , if not WARNING: Can't mass-assign protected attributes: role_ids
      attr_accessible :role_ids
     
      has_many :permissions
      has_many :roles, :through => :permissions
    end
    
    class Role < ActiveRecord::Base 
      attr_accessible :name, :users_count
      
      has_many :permissions
      has_many :users, :through => :permissions
      
    end
    
    class Permission < ActiveRecord::Base
      belongs_to :role, :counter_cache => true
      belongs_to :user
    end
    

    When i try to update my model (in a controller ) with

    @user.update_attributes(params[:user]

    i got this error

    rails.3.0.4rc/activerecord/lib/active_record/relation.rb:182:in update'<br/> rails.3.0.4rc/activerecord/lib/active_record/associations/has_many_through_association.rb:95:inblock in delete_records'
    rails.3.0.4rc/activerecord/lib/active_record/associations/has_many_through_association.rb:91:in each'<br/> rails.3.0.4rc/activerecord/lib/active_record/associations/has_many_through_association.rb:91:indelete_records'
    rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:222:in block in delete'<br/> rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:525:inblock in remove_records'
    rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:158:in block in transaction'<br/> rails.3.0.4rc/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:139:intransaction'
    rails.3.0.4rc/activerecord/lib/active_record/transactions.rb:207:in transaction'<br/> rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:157:intransaction'
    rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:522:in remove_records'<br/> rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:221:indelete'
    rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:359:in block in replace'<br/> rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:158:inblock in transaction'
    rails.3.0.4rc/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:139:in transaction'<br/> rails.3.0.4rc/activerecord/lib/active_record/transactions.rb:207:intransaction'
    rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:157:in transaction'<br/> rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:358:inreplace'
    rails.3.0.4rc/activerecord/lib/active_record/associations.rb:1524:in block in collection_accessor_methods'<br/> rails.3.0.4rc/activerecord/lib/active_record/associations.rb:1532:inblock in collection_accessor_methods'
    rails.3.0.4rc/activerecord/lib/active_record/base.rb:1558:in block in attributes='<br/> rails.3.0.4rc/activerecord/lib/active_record/base.rb:1554:ineach'
    rails.3.0.4rc/activerecord/lib/active_record/base.rb:1554:in attributes='<br/> rails.3.0.4rc/activerecord/lib/active_record/persistence.rb:127:inblock in update_attributes'
    rails.3.0.4rc/activerecord/lib/active_record/transactions.rb:292:in block in with_transaction_returning_status'<br/> rails.3.0.4rc/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:139:intransaction'
    rails.3.0.4rc/activerecord/lib/active_record/transactions.rb:207:in transaction'<br/> rails.3.0.4rc/activerecord/lib/active_record/transactions.rb:290:inwith_transaction_returning_status'
    rails.3.0.4rc/activerecord/lib/active_record/persistence.rb:126:in update_attributes'<br/> app/controllers/admin/users_controller.rb:141:inblock in update'
    rails.3.0.4rc/actionpack/lib/action_controller/metal/mime_responds.rb:264:in call'<br/> rails.3.0.4rc/actionpack/lib/action_controller/metal/mime_responds.rb:264:inretrieve_response_from_mimes'
    rails.3.0.4rc/actionpack/lib/action_controller/metal/mime_responds.rb:191:in respond_to'<br/> app/controllers/admin/users_controller.rb:140:inupdate'

    December 29th, 2010 @ 02:13 PM

  20. regis leray
    regis leray

    Nobody can look into this issue i m really stuck !

    Thanks in advance.

    look my previous stackstrace:

    rails.3.0.4rc/activerecord/lib/active_record/relation.rb:182:in update'

    rails.3.0.4rc/activerecord/lib/active_record/associations/has_many_through_association.rb:95:inblock in delete_records'

    rails.3.0.4rc/activerecord/lib/active_record/associations/has_many_through_association.rb:91:in each'

    rails.3.0.4rc/activerecord/lib/active_record/associations/has_many_through_association.rb:91:indelete_records'

    rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:222:in block in delete'

    rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:525:inblock in remove_records'

    rails.3.0.4rc/activerecord/lib/active_record/associations/association_collection.rb:158:in block in transaction'

    rails.3.0.4rc/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:139:intransaction'

    January 10th, 2011 @ 04:02 PM

  21. Repository
    Repository
    • State changed from open to resolved

    (from [52f09eac5b3d297021ef726e04ec19f6011cb302]) Correctly update counter caches on deletion for has_many :through [#2824 [Patch] has_many :through doesn't update counter_cache on join model correctly. state:resolved]. Also fixed a bunch of other counter cache bugs in the process, as once I fixed this one others started appearing like nobody's business. https://github.com/rails/rails/commit/52f09eac5b3d297021ef726e04ec1...

    February 7th, 2011 @ 11:54 PM

  22. Chris Masters
    Chris Masters

    Hi,

    Apologies if this is not the correct place to post this but I would like some advice on the best way to integrate this in my project.

    I am currently using rails 3.0.5 rather than rails edge.

    Should I change my gemfile to point to the git repository as I am deploying to heroku.

    Or is there an alternative? Perhaps cloning the git repository and manually copying the new files over?

    Or do I make a patch for 3.0.5?

    Apart from this functionality I am more than happy with everything else and thank you so much for all the hard work.

    Sorry about this being a n00b question and if there is an FAQ or similar I have missed with this information then I apologise.

    Many thanks,

    Chris

    March 28th, 2011 @ 10:38 AM