#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