Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #9167 (closed defect: fixed)

Opened 1 year ago

Last modified 1 year ago

[PATCH] HasManyAssociation ignores association-defined :include argument when counting records

Reported by: danger Assigned to: core
Priority: normal Milestone: 1.x
Component: ActiveRecord Version: edge
Severity: normal Keywords: tiny patch needs_review
Cc:

Description

When defining an association like the following: 

Author < ActiveRecord::Base
  has_many :posts_with_no_comments, :class_name => 'Post', :conditions => 'comments.id is null', :include => :comments
end

The select works fine. Records can be found with this method and managed as expected. What doesn't work is asking for the size of the association before it's been cached. 

@author.posts_with_no_comments.size # => ERROR
@author.posts_with_no_comments # => [#<Post ... >, #<Post ... >]

the count(*) is respecting the :conditions but ignoring :include so the SQL comes out bad.

I've attached a test that confirms this behavior.

Attachments

test_eager_with_multi_table_conditional_properly_counts_the_records.diff (2.3 kB) - added by danger on 08/03/07 21:28:02.
test_eager_with_conditional_properly_counts_the_records_stable.diff (2.3 kB) - added by danger on 08/15/07 14:04:24.
Patch for the 1-2 stable branch

Change History

08/02/07 10:06:45 changed by lifofifo

You should use @author.posts_with_no_comments.length instead of .size to force loading of association.

I'm lost when you say count is ignoring :include. Could you please elaborate ?

Thanks.

08/02/07 20:27:19 changed by danger

lifofifo: I didn't realize length would force the load. That's a good hack for my current project but I'd rather not have to load all the rows into memory just to count them.

Regarding how count is ignoring include:

SQL for select: 

SELECT posts.`id` AS t0_r0, posts.`author_id` AS t0_r1, posts.`title` AS t0_r2, posts.`body` AS t0_r3, posts.`type` AS t0_r4, posts.`taggings_count` AS t0_r5, comments.`id` AS t1_r0, comments.`post_id` AS t1_r1, comments.`body` AS t1_r2, comments.`type` AS t1_r3 FROM posts  LEFT OUTER JOIN comments ON comments.post_id = posts.id WHERE (posts.author_id = 1 AND (comments.id is null))

SQL for count: 

SELECT count(*) AS count_all FROM posts WHERE (posts.author_id = 1 AND (comments.id is null))

08/03/07 21:28:02 changed by danger

  • attachment test_eager_with_multi_table_conditional_properly_counts_the_records.diff added.

08/03/07 21:30:29 changed by danger

  • summary changed from Multi-table association ignores :include argument when counting records to [PATCH] HasManyAssociation ignores :include argument when counting records.

It turns out it was a tiny fix. When the association finally hands off @counter_sql to count() it forgets to add: :include => @reflection.options[:include].

43 characters for the code patch. Test included.

08/03/07 21:33:44 changed by lifofifo

Thanks for explanation.

Could you please post your previous fix in comments ?

08/03/07 21:53:04 changed by danger

I'm not sure what you mean by 'previous fix'. The only fix I've found is adding :include => @reflection.options[:include]. You can see it in more detail in the patch.

08/03/07 22:08:40 changed by lifofifo

Oops..my bad. That comment wasn't meant for this ticket. Nevermind.

08/03/07 22:11:39 changed by danger

I do that all the time ;)

08/07/07 23:09:03 changed by danger

  • keywords set to tiny patch.

08/15/07 14:04:24 changed by danger

  • attachment test_eager_with_conditional_properly_counts_the_records_stable.diff added.

Patch for the 1-2 stable branch

08/15/07 14:07:39 changed by danger

  • keywords changed from tiny patch to tiny patch needs_review.
  • summary changed from [PATCH] HasManyAssociation ignores :include argument when counting records to [PATCH] HasManyAssociation ignores association-defined :include argument when counting records.

08/16/07 04:36:58 changed by nzkoz

  • status changed from new to closed.
  • resolution set to fixed.

(In [7325]) Make sure has_many associations honour :include when counting. Closes #9167 [danger]

08/16/07 04:58:09 changed by nzkoz

(In [7327]) Merge [7235] to stable. References #9167 [danger]