Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Counter cache doesn't use cache when classname can't be pluralized #7993

Closed
vladislav-rvd opened this Issue · 4 comments

6 participants

Vladislav Rovda Matt Jones Steve Klabnik Yves Senn Matthew Robertson Jon Leighton
Vladislav Rovda
class Classroom < ActiveRecord::Base
  has_many :classroomship, :dependent => :destroy
end

class Classroomship < ActiveRecord::Base
  set_table_name 'classrooms_parentum'
  belongs_to :classroom, :counter_cache => :classrooms_parentum_count
end

class AddClassroomsParentumCountToClassroom < ActiveRecord::Migration
  def self.up
    add_column :classrooms, :classrooms_parentum_count, :integer, :default => 0
  end
end

classroom = Classroom.last
classroom.classroomship.size
 ->  (0.5ms)  SELECT COUNT(*) FROM `classrooms_parentum` WHERE `classrooms_parentum`.`classroom_id` = 109
 => 6 

But when I add new classroomship classrooms_parentum_count will be updated

classroom.classroomship.create(:parentis => Parentis.first)
-> SQL (0.2ms)  UPDATE `classrooms` SET `classrooms_parentum_count` = COALESCE(`classrooms_parentum_count`, 0) + 1 WHERE `classrooms`.`id` = 109

Rails 3.1.3

Matt Jones

This doesn't look like a pluralization issue exactly - the problem is that the code to look up counter cache column to use makes an incorrect assumption about what the name of the column should be:

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/has_many_association.rb#L78

It uses #{reflection.name}_count (classroomship_count in this example) instead of the value configured on the belongs_to.

At a minimum, we should probably add an option to has_many to select which counter cache column to use.

Steve Klabnik
Collaborator

Okay, so that means it appears present on master as well.

@vladislav-rvd , 3.1 isn't supported any longer, you should upgrade as soon as you can.

Yves Senn
Owner

I attached a PR, which introduces the :counter_cache option to has_many as @al2o3cr described.

Matthew Robertson

@jonleighton @senny I'm leaning into a few counter_cache bugs right now and this commit seems like it might be counter productive. See issues: #7630 #3903. I will try to write up a more thorough explanation when I have it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.