This project is archived and is in readonly mode.

#646 ✓stale
Smilinguy

eager loading duplicates conditions unnecessarily

Reported by Smilinguy | July 17th, 2008 @ 08:08 PM | in 2.x

I noticed that when querying for limited numbers of results, that the eager loading of objects with have has_and_belongs_to_many and has_many :through associations appears to run a pre-query to obtain object ids then performs the eager loading of those objects by id with the included associations.

The issue I saw was that the conditions block was run on both the pre-query for IDs and the eager loading query. Perhaps I'm missing something, but if you already have the object ids, and you are really just querying by ID why do you need to run the conditions again? It slows things down and it looks to me to be unnecessary.

I'm not sure how to make a patch, but the method that I think needs to be changed is construct_finder_sql_with_included_associations.

I changed the lines

add_conditions!(sql, options[:conditions], scope)
add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) 

to

if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
            add_limited_ids_condition!(sql, options, join_dependency)
          else
            add_conditions!(sql, options[:conditions], scope)
          end  

Not sure what other impacts this will have or if I'm correct in my assumptions. Any comments or feedback would be appreciated.

Comments and changes to this ticket

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>

Pages