count on HABTMs broken by Ruby 1.8.7 Array.count
Reported by Mike Champion | August 14th, 2008 @ 05:05 PM | in 2.2
Ruby 1.8.7 introduces several new methods including Array.count (through Enumerable). This appears to break using the count method on HABTM relationships, although not has_many relationships.
I have a test rails app that I have tested with ruby 1.8.7p22 against Rails 2.1 and edge Rails. Both exhibit this problem.
The models are:
class Author < AR::Base
has_and_belongs_to_many :posts
end
class Post < AR::Base
has_and_belongs_to_many :authors
end
In the unit test, there is:
# This author has 2 posts
mike = authors(:mike)
# Appears to be correct, but looking at logs it is not
# doing a "count(*)" but instead is doing full "select (*)" via
# Array.count
assert_equal 2, mike.posts.count
# This generates:
# ArgumentError: wrong number of arguments (2 for 1)
# because it calls Array.count instead of AR calculations.
assert_equal 2, mike.posts(true).count(:all, :conditions => ["posts.created_at < ?", Time.utc('2020')])
I haven't dug into the Rails sources much to see why this is only happening for HABTM associations, but was wondering if others had encountered this and hopefully had a fix.
Comments and changes to this ticket
-
Ernie Miller August 14th, 2008 @ 09:14 PM
- → Tag changed from “2.1 activerecord edge” to “2.1 activerecord bug edge patch”
There's actually no count method defined on the habtm association class, which would help explain the issue. ;)
After a quick look through the association code, I think what's happening is that without an Array#count (1.8.6) AssociationCollection#method_missing is calling the reflection class's count method scoped to its owner.
With an Array#count (1.8.7), however, the first condition of AssociationCollection#method_missing is being met and instead the call is passed up the line to AssociationProxy, which in turn passes it to the @target directly, after loading it.
There are two possible ways this could be resolved. It seems that the way that most keeps in line with the other association types is to implement HasAndBelongsToManyAssociation#count. Another shortcut would be to change AssociationCollection#method_missing such that the first condition won't pass if the method is :count.
The former isn't very DRY, and the latter feels awfully hackish to me.
So instead, I've opted too try something much more dangerous, and refactored the handling of count so that it's implemented inside AssociationCollection instead of its descendants.
This is really rough, but could you give the attached patch a try and see if it resolves the issue? I'm not running 1.8.7 here, because I'm stuck on a Windows box at the moment with a One-Click-Installer Ruby installation. I can at least confirm that nothing new in the ActiveRecord MySQL test suite is failing from this patch, but I'm not sure there's ample test coverage for all of the various :counter_sql and :finder_sql possibilities in all 3 affected association macros.
Assuming positive feedback, I'll update docs and add any tests that might be needed as well, since :counter_sql doesn't seem to be documented everywhere, and wasn't being used on HABTM associations before.
-
Mike Champion August 14th, 2008 @ 09:27 PM
Thanks. I think you're right as to the cause. The method_missing in association_collection looks at the class level methods, where "count" is defined by calculations.rb. It works for HasMany relationships because that defines a count method on the instance and overrides the Array.count, I believe, whereas HABTM doesn't define it.
I started down the path of adding a count method similar to what is in has_many_association to has_and_belongs_to_many_association. Although maybe I will just try to have it restore the behavior of what it was doing for ruby 1.8.6.
-
Ernie Miller August 14th, 2008 @ 09:38 PM
If you get the chance, try the patch I attached and let me know how it works for you... and if it breaks anything new and exotic! ;)
-
Ernie Miller August 15th, 2008 @ 03:30 AM
Updated patch to add a bit of documentation.
I'm a little concerned about the handling of a has_many association's :conditions option if passed to count. Before the refactor, it was pulling out the conditions ahead of time and modifying the association's @finder_sql. I think this is because it wasn't taking advantage of with_scope, and as I said before, all tests continue to pass, so I think the refactor went off without a hitch... But if someone can show a test case where this patch breaks things, I'd like to include that test and fix the bug.
-
Mike Champion August 20th, 2008 @ 07:31 PM
Ernie, I applied your patch and it fixed my small sample app's tests that exercised the HABTM count problem. Thanks!
I have not yet tried it in our real app to see if other issues are exhibited.
-
Ernie Miller August 28th, 2008 @ 07:10 PM
- → Tag changed from “2.1 activerecord bug edge patch” to “2.1 activerecord bug edge patch tests”
Added a test based on Mike's example above, and merged Tarmo's :offset and :limit constraints from #348 as well.
-
Jeremy Kemper August 28th, 2008 @ 08:04 PM
- → Milestone changed from “2.x” to “2.2”
- → State changed from “new” to “open”
- → Assigned user changed from “” to “Jeremy Kemper”
-
Jeremy Kemper August 28th, 2008 @ 08:04 PM
Doesn't apply cleanly to 2-1-stable so this'll be 2.2 only.
-
Repository August 28th, 2008 @ 08:05 PM
- → State changed from “open” to “resolved”
(from [44af2efa2c7391681968c827ca47201a0a02e974]) Refactored AssociationCollection#count for uniformity and Ruby 1.8.7 support.
[#831 state:resolved]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net http://github.com/rails/rails/co...
-
-
Tekin September 24th, 2008 @ 01:50 PM
- → Tag changed from “2.1 activerecord bug edge patch tests” to “2.1 activerecord bug edge patch tests”
:counter_sql has not been added to the valid keys hash in create_has_and_belongs_to_many_reflections on line 1574 of associations.rb so :counter_sql is being rejected.
-
Ernie Miller September 24th, 2008 @ 02:02 PM
Good catch. This is a trivial fix -- Jeremy, do you want me to submit another patch, or is this fixed in edge already?
-
Ernie Miller September 24th, 2008 @ 02:40 PM
I checked, and it hasn't been fixed in edge yet. See #1102 for the one-liner patch. We should really get some fixtures that use counter_sql on a HABTM to catch this in the future, but I don't have time to knock that out today and since David mentioned 2.2 beta was coming soon, I didn't want this simple bug sneaking into the beta if it could be helped.
-
Michael Simons November 5th, 2008 @ 08:44 PM
This problem applies to 2.1.2 as well.
I confirm this for Debian/Lenny, 2.6.26-1-686 #1 SMP, with ruby 1.8.7 (2008-08-11 patchlevel 72) [i486-linux] and Rails 2.1.2
HABTM is:
has_and_belongs_to_many :friends, :class_name => 'User', :join_table => 'friends', :foreign_key => 'user_id', :association_foreign_key => 'friend_id', :order => 'NAME ASC, VORNAME ASC'Query is
friends.count(:conditions => ["friend_id = :user_id", {:user_id => user_id}])Any help (and fixing) is appreciated!
I really like to go to 2.2 but i can't (and i guess a lot of people that are stuck to gettext for the time being can't either)
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
Source available from github
Repository is at http://github.com/rails/rails
Check out the development master (Edge Rails):
git clone git://github.com/rails/rails.git
Creating or reviewing a patch
See the contributor guide.
Creating a feature request
Please don't. If you want a new feature in Rails, you'll have to pull up your sleeves and get busy yourself. Or convince someone else to do it. See the contributor guide on how to get going. But posting them here is just going to lead to ticket root.
Creating a bug report
When creating a bug report, be sure to include as much relevant information as possible. Post the code sample that causes the problem. Preferably, alter the unit tests and show through either changed or added tests how the expected behavior is not occuring.
Security vulnerabilities should be reported via an email to security@rubyonrails.org, do not use trac for reporting security vulnerabilities. All content in trac is publicly available as soon as it is posted.
Then don't get your hopes up. Unless you have a "Code Red, Mission Critical, The World is Coming to an End" kinda bug, you're creating this ticket in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the ticket automatically will see any activity or that others will jump to fix it. Creating a ticket like this is mostly to help yourself start on the path of fixing the problem and for others to sign on to with a "I'm having this problem too"..
