This project is archived and is in readonly mode.
Custom counter_cache is not used when using collection.size method
Reported by Szymon Nowak | July 14th, 2009 @ 12:02 AM | in 2.3.10
While writing a patch for belongs_to association, I've
encountered the following issue:
class Author < AR::Base
has_many :posts
end
class Post < AR::Base
belongs_to :author, :counter_cache => "custom_posts_count"
end
a = Author.create
p = Post.create
p.author = a # updates counter on Author, but doesn't save post object
Author.find(a.id).send(:read_attribute, "custom_posts_count" # 1; uses counter cache
Author.find(a.id).posts.size # 0; uses SQL count
In the second example SQL count is used because in
HasManyAssociation#count_records method, there's check if there's
cache counter:
def has_cached_counter?
@owner.attribute_present?(cached_counter_attribute_name)
end
def cached_counter_attribute_name
"#{@reflection.name}_count"
end
which fails in this case. It simply uses name of the reflection,
not the value of :counter_cache option on belongs_to association,
or table name as described in
docs.
Not sure if there's an easy solution for this if counter cache column name is defined on belongs_to association and not has_many.
Comments and changes to this ticket
-
Jeremy Kemper July 15th, 2009 @ 10:21 PM
- Milestone changed from 2.x to 2.3.4
- State changed from new to open
-
Jeremy Kemper September 11th, 2009 @ 11:04 PM
- Milestone changed from 2.3.4 to 2.3.6
[milestone:id#50064 bulk edit command]
-
Balint Erdi January 13th, 2010 @ 04:48 PM
I think this is not related with the counter_cache column having the default name or a custom one. I found two tests for this:
def test_counter_cache topic = Topic.create :title => "Zoom-zoom-zoom" assert_equal 0, topic[:replies_count] reply = Reply.create(:title => "re: zoom", :content => "speedy quick!") reply.topic = topic assert_equal 1, topic.reload[:replies_count] assert_equal 1, topic.replies.size topic[:replies_count] = 15 assert_equal 15, topic.replies.size end def test_custom_counter_cache reply = Reply.create(:title => "re: zoom", :content => "speedy quick!") assert_equal 0, reply[:replies_count] silly = SillyReply.create(:title => "gaga", :content => "boo-boo") silly.reply = reply assert_equal 1, reply.reload[:replies_count] assert_equal 1, reply.replies.size reply[:replies_count] = 17 assert_equal 17, reply.replies.size end
Both of these pass, of course. Now if you comment out the line that reloads the "has-many end" of the association,
# assert_equal 1, topic.reload[:replies_count]
and
# assert_equal 1, reply.reload[:replies_count]
then both of them fail.
Now I guess that the tests serve as specifications, too, so that the collection size should only show the updated value through the counter_cache after a reload. If this is not the case, then probably both cases (tests) have to be redefined and fixed.
-
Balint Erdi January 13th, 2010 @ 05:55 PM
Hmm, but I did manage to recreate the original problem:
With :counter_cache => true :
>> Author.find(a.id).send(:read_attribute, "posts_count") => 1 >> Author.find(a.id).posts.size => 1
With :counter_cache => "custom_posts_count":
>> Author.find(a.id).send(:read_attribute, "custom_posts_count") => 1 >> Author.find(a.id).posts.size => 0
But failed to write a failing unit test at this point.
-
Balint Erdi January 15th, 2010 @ 12:55 PM
- Tag changed from active_record, collection, counter_cache to active_record, collection, counter_cache, patch
-
Balint Erdi January 16th, 2010 @ 12:35 PM
@Fernando, I am really happy to hear that, but beyond trust, could you check out the 2-3-stable branch, apply my patch, and tell me if it works? Only if you have the time, of course. Thank you!
-
Balint Erdi January 18th, 2010 @ 12:50 PM
- Tag changed from active_record, collection, counter_cache, patch to 2.3.6, active_record, collection, counter_cache, patch
-
Rizwan Reza May 16th, 2010 @ 02:41 AM
- Tag changed from 2.3.6, active_record, collection, counter_cache, patch to 2.3.6, active_record, bugmash, collection, counter_cache, patch
-
Tanja Otto May 16th, 2010 @ 08:58 PM
adapted testcases of Balint Erdi because apply of his diff doesn't work in master. I’ve attached a patch.
-
Tanja Otto May 16th, 2010 @ 09:19 PM
adapted fix of Balint Erdi because apply of his diff doesn't work in master. I’ve attached a patch.
-
Hussein Morsy May 17th, 2010 @ 08:38 AM
+1 verified. The testcase and the fix of Tanja works for the master (3.x)
-
Jeremy Kemper August 30th, 2010 @ 02:28 AM
- Milestone changed from 2.3.9 to 2.3.10
- Importance changed from to
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>
People watching this ticket
Attachments
Referenced by
- 765 PATCH: primary_key option for belongs_to Here's hopefully complete patch. I tested assignments, co...