This project is archived and is in readonly mode.
3.0.5 introduced repeated column lookups for eager loaded has_one with conditions
Reported by Patrick Bacon | March 20th, 2011 @ 03:38 PM | in 3.0.6
This commit appears to have (unintentionally?) removed some caching that was being done for table columns when a has_one (possibly other types as well) association has conditions and is eager loaded.
I am running into a situation where several hundred rows are being selected, with an eager loaded has_one that has conditions. The columns for the associated table are being queried (at least) once for each result row. This did not happen in v3.0.4. And it makes the query unacceptably slow.
It appears that when eager loading a has_one
association an AssociationProxy
instance is created
for each result record, and its @finder_sql
instance
variable is populated in its constructor. If the association has
conditions they need to be sanitized against the columns for the
table being associated with.
The commit linked above introduces a new, preferred way of
specifying interpolated conditions on associations (using a proc).
In doing so the sanitized_conditions
method of the
MacroReflection
class is no longer being used, and
that is where the caching used to take place.
From the AssociationProxy
class in v3.0.4:
def conditions
@conditions ||= interpolate_sql(@reflection.sanitized_conditions) if @reflection.sanitized_conditions
end
And here it is in 3.0.5 (no longer using
sanitized_conditions
):
def conditions
@conditions ||= @reflection.options[:conditions] && interpolate_and_sanitize_sql(@reflection.options[:conditions])
end
The problem can be seen in a test by adding a has_one with conditions to the test model Post class (the following test stuff is also attached as a patch against the 3-0-stable branch):
has_one :first_post_comment, :class_name => 'Comment', :conditions => {:body => 'First Post!'}
In order to track the column lookups for a specific table I
modified the column_with_calls
code in
test/cases/helper.rb
:
ActiveRecord::Base.connection.class.class_eval {
attr_accessor :column_calls, :column_calls_by_table
def columns_with_calls(*args)
@column_calls ||= 0
@column_calls_by_table ||= Hash.new {|h,table| h[table] = 0}
@column_calls += 1
@column_calls_by_table[args.first.to_s] += 1
columns_without_calls(*args)
end
alias_method_chain :columns, :calls
}
And finally a test added to
test/cases/associations/eager_test.rb
. This test
passes in 3.0.4 and fails in 3.0.5:
def test_preload_has_one_with_conditions
# pre-heat our cache
Post.arel_table.columns
Comment.columns
Post.connection.column_calls_by_table['comments'] = 0
Post.includes(:very_special_comment).all.to_a
assert_equal 0, Post.connection.column_calls_by_table['comments']
Post.connection.column_calls_by_table['comments'] = 0
Post.includes(:first_post_comment).all.to_a
assert_equal 1, Post.connection.column_calls_by_table['comments']
end
I have not worked out a fix for this yet.
Comments and changes to this ticket
-
Patrick Bacon March 30th, 2011 @ 03:19 AM
I have submitted a pull request with an attempted fix: https://github.com/rails/rails/pull/246
-
Aaron Patterson March 30th, 2011 @ 11:43 PM
- State changed from new to committed
- Milestone set to 3.0.6
- Assigned user set to Aaron Patterson
- Importance changed from to Low
Merged, thanks!
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>