This project is archived and is in readonly mode.

#1860 open
Anselm Helbig

with_scope should merge complex includes

Reported by Anselm Helbig | February 3rd, 2009 @ 03:46 PM | in 3.0.6

with_scope' fails to merge complex includes. Take this code:

Article.with_scope(:find => { :include => :comments }) do
  Article.with_scope(:find => { :include => { :comments => :users } }) do
    Article.find(:all)
  end
end

The includes in effect are [:comments, { :comments => :users }] instead of [{ :comments => :users }] which might result in an unnecessary join.

Responsible for this behaviour is the code in ActiveRecord::Base::merge_includes - it should be able to recognize such complex includes and merge them accordingly.

Comments and changes to this ticket

  • Anselm Helbig
  • Brian Rose

    Brian Rose April 12th, 2010 @ 11:51 PM

    • Tag set to activerecord, include, joins
    • Assigned user set to “Ryan Bigg”

    Unable to replicate under Rails 3.0. The following code only produces three queries (one for each table):

    Article.includes(:comments => :user).includes(:comments).all

  • Ryan Bigg

    Ryan Bigg April 13th, 2010 @ 12:39 AM

    • State changed from “new” to “resolved”
    • Milestone cleared.

    I agree, this is more than likely fixed in Rails 3.

  • Ryan Bigg

    Ryan Bigg October 11th, 2010 @ 12:11 PM

    • Tag cleared.
    • Importance changed from “” to “Low”

    Automatic cleanup of spam.

  • Brian Underwood

    Brian Underwood October 13th, 2010 @ 10:11 PM

    I think that this is still happening in Rails 3. It've been struggling with this for a while. Could it be re-opened?

    It shows up when you use a where/order which causes Rails to create a join query rather than using separate queries for the eager loading.

    Here's an example in my app that is failing:
    Change.includes(:user).includes(:user => :regions).where('users.deleted_at IS NULL').all

    The application uses Postgres, so I get the following error:

    ActiveRecord::StatementInvalid: PGError: ERROR: missing FROM-clause entry for table "users_changes"
    LINE 1: ..." AS t1_r18, "users"."email_signature" AS t1_r19, "users_cha...

    Of course the "users_changes" table doesn't exist as "user" is a has_many association on the Change model.

    In the SQL query, the "changes", "users", and "regions" tables are used among the select fields. "changes", "users", "regions_users", and "regions" are all joined in the FROM clause (see attachment for the full error including the SQL query and backtrace)

    Using Rails 3.0.0 and RVM on OS 10.6 with ruby version "ruby 1.8.7 (2010-08-16 patchlevel 302) [i686-darwin10.4.0]"

  • Brian Underwood

    Brian Underwood October 13th, 2010 @ 10:36 PM

    Perhaps not surprisingly, Change.includes(:user).includes(:user => :regions).where('users.deleted_at IS NULL').size works, probably because there are no fields specified in the SELECT clause but rather a COUNT(DISTINCT "changes"."id"). Here is the query that in generates:

    SQL (51.7ms) SELECT COUNT(DISTINCT "changes"."id") AS count_id FROM "changes" LEFT OUTER JOIN "users" ON "users"."id" = "changes"."user_id" LEFT OUTER JOIN "regions_users" ON "regions_users"."user_id" = "users"."id" LEFT OUTER JOIN "regions" ON "regions"."id" = "regions_users"."region_id" WHERE (users.deleted_at IS NULL)

  • Brian Underwood

    Brian Underwood October 13th, 2010 @ 10:44 PM

    Digging around a bit myself, it looks like the "includes_values" instance_variable is getting set to [:user, {:user=>:regions}]. Is that right?

    I found that variable being used in ActiveRecord::FinderMethods#find_with_associations

    Searching for the string "includes_values", the only place that I see it being assigned is in ActiveRecord::QueryMethods#includes

    Am I on the right track (I'm also about to get off the train and my connection is slow)?

  • Brian Underwood

    Brian Underwood October 14th, 2010 @ 12:29 AM

    I've done some more digging and I've created a patch which seems to work. Please have a look.

  • Brian Underwood

    Brian Underwood October 14th, 2010 @ 02:36 PM

    I've made a better patch including tests. Please have a look and thanks!

  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2
  • Brian Underwood

    Brian Underwood October 18th, 2010 @ 02:23 PM

    Just curious: what happens to this ticket now? Should it be marked as open again? Does the milestone changing to 3.0.2 mean that it will go in, or that it targeted for that version? Thanks!

  • Brian Underwood

    Brian Underwood October 18th, 2010 @ 02:25 PM

    Sorry for being so persistent. It's my first patch and I'm a bit caught up in the excitement of open source code in action ;)

  • Aditya Sanghi

    Aditya Sanghi October 18th, 2010 @ 02:37 PM

    • State changed from “resolved” to “open”
    • Tag set to activerelation, active_record, patch

    Hi Brian,

    No it doesnt mean that explicitly, i think that was just a bulk update.

    Adding a patch tag to the ticket and marking it open for now since you can reproduce the problem on Rails 3. Someone will need to validate that the patch applies and comment/code review etc. It may then end up in Rails.

    I've not validated your patch but maybe i'll give it a shot as soon as i get the time.

    I understand your enthusiasm, i was in a similar boat a few weeks ago! Keep it up!

  • Anatoliy Lysenko

    Anatoliy Lysenko October 25th, 2010 @ 01:44 PM

    Hi Brian,
    Аccidentally I and Ernie Miller fix this error when we work on #5845.
    We have two patches, hope you'll review them both.
    Unfortunately I dislike your patch, it fail because loop isn't right way to work with associations.
    Here I added test that fail with you patch:
    https://rails.lighthouseapp.com/projects/8994/tickets/5845/a/735636...
    Your patch is good for [:price_estimates,{:price_estimates => :pirate} where loop is good enough. But it fail on [{:posts=>:author}, {:posts=>:comments}] where you should use recursion, like in JoinDependecy#build.

  • Brian Underwood

    Brian Underwood October 25th, 2010 @ 06:38 PM

    Thanks! I'm just glad to see that it's going to be fixed ;) I've got my patch frozen into our version of 3.0.1 in our app, but I look forward to having your patch in soon (and being able to merge hashes in includes)!

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer November 1st, 2010 @ 05:06 PM

    • Tag cleared.

    Automatic cleanup of spam.

  • Santiago Pastorino
  • Santiago Pastorino
  • Santiago Pastorino

    Santiago Pastorino February 27th, 2011 @ 03:15 AM

    • Milestone changed from 3.0.5 to 3.0.6
  • klkk

    klkk May 23rd, 2011 @ 02:53 AM

    louisvuittonwarehouse.com

  • klkk

Create your profile

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

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages