This project is archived and is in readonly mode.
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 February 3rd, 2009 @ 04:45 PM
- no changes were found...
-
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 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 October 11th, 2010 @ 12:11 PM
- Tag cleared.
- Importance changed from “” to “Low”
Automatic cleanup of spam.
-
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').allThe 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 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 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 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 October 14th, 2010 @ 02:36 PM
I've made a better patch including tests. Please have a look and thanks!
-
-
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 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 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 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 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)!
-
-
-
-
Santiago Pastorino February 27th, 2011 @ 03:15 AM
- Milestone changed from “3.0.5” to “3.0.6”
-
-
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>
People watching this ticket
Anatoliy Lysenko
Jeremy Kemper
Ryan Bigg
Attachments
Referenced by
-
5845
ActiveRecord 3 eager loading fail
Ernie,
I doesn't completely understand your last comment....
-
5845
ActiveRecord 3 eager loading fail
Ernie,
I doesn't completely understand your last comment....
-
5353
ActiveRecord adds comma to query's column list when referring to a model with a default_scope select()
Also, you might be able to close #1860 as
well, I believe...