Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

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

#198

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?

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

State: resolved
Milestone: none
Assigned to: Mislav Mislav

Activity

  1. Denis Barushev
    Denis Barushev
    • 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.

    May 15th, 2008 @ 09:02 PM

  2. Mislav
    Mislav
    • 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!

    May 16th, 2008 @ 12:27 PM

  3. Denis Barushev
    Denis Barushev

    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?

    May 16th, 2008 @ 12:48 PM

  4. Mislav
  5. Peter
    Peter
    • 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.

    September 3rd, 2008 @ 07:05 PM

  6. Peter
    Peter

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

    September 3rd, 2008 @ 07:15 PM

  7. Mislav
    Mislav
    • State changed from open to resolved

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

    September 13th, 2008 @ 02:07 AM

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.