#198 ✓resolved
Denis Barushev

Removing :include option from count query when it's possible

Reported by Denis Barushev | March 7th, 2008 @ 12:56 AM

In http://dev.rubyonrails.org/chang... was introduced new preload query strategy for eager :includes. Now Rails Edge uses one SELECT query per table (one for model table and one for each include table) instead a monster query with joins if it's possible. This strategy automatically are not used when there are conditions for included tables or something else (I'm not ruby/rails hacker yet):

activerecord/lib/active_record/base.rb:1242

if include_associations.any? && references_eager_loaded_tables?(options)
  records = find_with_associations(options)
else
  records = find_by_sql(construct_finder_sql(options))
  if include_associations.any?
    preload_associations(records, include_associations)
  end
end

For example:

class Post < ActiveRecord::Base
  has_many :comments
end

class Comment < ActiveRecord::Base
end
Post.find :all, :include => :comments

SELECT * FROM "posts"
SELECT comments.* FROM "comments" WHERE (comments.post_id IN (1,2))
Post.find :all, :include => :comments, :conditions => "posts.id > 1"

SELECT * FROM "posts" WHERE (posts.id > 1)
SELECT comments.* FROM "comments" WHERE (comments.post_id IN (2))
Post.find :all, :include => :comments, :conditions => "comments.id" > 1

SELECT "posts"."id" AS t0_r0, "posts"."title" AS t0_r1, "posts"."created_at" AS t0_r2, "posts"."updated_at" AS t0_r3, "comments"."id" AS t1_r0, "comments"."body" AS t1_r1, "comments"."post_id" AS t1_r2, "comments"."created_at" AS t1_r3, "comments"."updated_at" AS t1_r4 FROM "posts" LEFT OUTER JOIN "comments" ON comments.post_id = posts.id WHERE (comments.id > 1)
Post.paginate, :include => :comments

are two calls with some additional params(:offset, :linit) in fact:

Post.find, :include => :comments
Post.count, :include => :comments

But count method doesn't use this eager strategy. This is logically.

When I use paginate method with :include option, count method generated by it ALWAYS are invoked with param when it doesn't need.

For example,

Post.paginate :include => :comments, :page => 1

generates queries:

SELECT * FROM "posts"
SELECT comments.* FROM "comments" WHERE (comments.post_id IN (1,2))
SELECT count(DISTINCT "posts".id) AS count_all FROM "posts" LEFT OUTER JOIN "comments" ON comments.post_id = posts.id

but, in this case, last query may be:

SELECT count(*) AS count_all FROM "posts"

Main task is modify WillPaginate::Finder::ClassMethods#wp_count for distinguishing when it should add "inlude" to "excludes" array. I can't do that. May be anybody can examine how that works in 'find' method?

Comments and changes to this ticket

  • Denis Barushev

    Denis Barushev May 15th, 2008 @ 09:02 PM

    • Title changed from “"#paginate :include => something": removing joins from count query when it's possible” to “[PATCH] Removing :include option from count query when it's possible”

    I've done this. See patch.

  • Mislav

    Mislav May 16th, 2008 @ 12:27 PM

    • State changed from “new” to “open”
    • Title changed from “[PATCH] Removing :include option from count query when it's possible” to “Removing :include option from count query when it's possible”

    This looks good, but next time please make a test :)

    Also, with Lighthouse "patch" tag we don't need "[PATCH]" in subject anymore (that was a Trac thing).

    Thanks!

  • Denis Barushev

    Denis Barushev May 16th, 2008 @ 12:48 PM

    I now about test, but there are some problems. This patch is about internals of #wp_count. I dunno how to test this. Test generated query is not good idea and I also dunno how to do this. May be it's good time for extracting #prepare_count_options from #wp_count?

  • Mislav
  • Peter

    Peter September 3rd, 2008 @ 07:05 PM

    • Tag set to count, patch, will_paginate

    I just applied this patch but it is still doing the include in the count_all breaking my query because it's also doing an INNER JOIN duplicating a table name.

  • Peter

    Peter September 3rd, 2008 @ 07:15 PM

    Nevermind, it was part of my model association. I actually don't need the patch, thanks.

  • Mislav

    Mislav September 13th, 2008 @ 02:07 AM

    • State changed from “open” to “resolved”

    This has been long resolved (fix present in both master and agnostic branch)

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Everyone's favorite Ruby library for pagination of practically anything!

Attachments

Pages