Skip to content

Commit

Permalink
Added back the use of the Reflection module's cached sanitized_condit…
Browse files Browse the repository at this point in the history
…ions in an AssociationProxy. This was recently removed and when a has_one association with conditions is eager loaded the conditions would be sanitized once for every result row, causing a database hit to fetch the columns.
  • Loading branch information
baconpat authored and tenderlove committed Mar 30, 2011
1 parent 05f3df3 commit d6dbd54
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 4 deletions.
Expand Up @@ -102,7 +102,7 @@ def aliased_table_name
# Returns the SQL string that corresponds to the <tt>:conditions</tt> # Returns the SQL string that corresponds to the <tt>:conditions</tt>
# option of the macro, if given, or +nil+ otherwise. # option of the macro, if given, or +nil+ otherwise.
def conditions def conditions
@conditions ||= @reflection.options[:conditions] && interpolate_and_sanitize_sql(@reflection.options[:conditions]) @conditions ||= interpolate_sanitized_sql(@reflection.sanitized_conditions) if @reflection.sanitized_conditions
end end
alias :sql_conditions :conditions alias :sql_conditions :conditions


Expand Down Expand Up @@ -161,6 +161,10 @@ def dependent?
@reflection.options[:dependent] @reflection.options[:dependent]
end end


def interpolate_sanitized_sql(sql, record = nil, sanitize_klass = @reflection.klass)
@owner.send(:interpolate_sanitized_sql, sql, record, sanitize_klass)
end

def interpolate_and_sanitize_sql(sql, record = nil, sanitize_klass = @reflection.klass) def interpolate_and_sanitize_sql(sql, record = nil, sanitize_klass = @reflection.klass)
@owner.send(:interpolate_and_sanitize_sql, sql, record, sanitize_klass) @owner.send(:interpolate_and_sanitize_sql, sql, record, sanitize_klass)
end end
Expand Down
7 changes: 5 additions & 2 deletions activerecord/lib/active_record/base.rb
Expand Up @@ -1732,16 +1732,19 @@ def quote_value(value, column = nil)


def interpolate_and_sanitize_sql(sql, record = nil, sanitize_klass = self.class) def interpolate_and_sanitize_sql(sql, record = nil, sanitize_klass = self.class)
sanitized = sanitize_klass.send(:sanitize_sql, sql) sanitized = sanitize_klass.send(:sanitize_sql, sql)
interpolate_sanitized_sql(sanitized, record, sanitize_klass)
end


def interpolate_sanitized_sql(sanitized, record = nil, sanitize_klass = self.class)
if sanitized =~ /\#\{.*\}/ if sanitized =~ /\#\{.*\}/
ActiveSupport::Deprecation.warn( ActiveSupport::Deprecation.warn(
'String-based interpolation of association conditions is deprecated. Please use a ' \ 'String-based interpolation of association conditions is deprecated. Please use a ' \
'proc instead. So, for example, has_many :older_friends, :conditions => \'age > #{age}\' ' \ 'proc instead. So, for example, has_many :older_friends, :conditions => \'age > #{age}\' ' \
'should be changed to has_many :older_friends, :conditions => proc { "age > #{age}" }.' 'should be changed to has_many :older_friends, :conditions => proc { "age > #{age}" }.'
) )
instance_eval("%@#{sanitized.gsub('@', '\@')}@", __FILE__, __LINE__) instance_eval("%@#{sanitized.gsub('@', '\@')}@", __FILE__, __LINE__)
elsif sql.respond_to?(:to_proc) elsif sanitized.respond_to?(:to_proc)
sanitize_klass.send(:sanitize_sql, instance_exec(record, &sql)) sanitize_klass.send(:sanitize_sql, instance_exec(record, &sanitized))
else else
sanitized sanitized
end end
Expand Down
17 changes: 17 additions & 0 deletions activerecord/test/cases/associations/eager_test.rb
Expand Up @@ -670,6 +670,23 @@ def test_preload_with_deprecated_interpolation
assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
end end


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

# Don't care exactly how many column lookup are done,
# as long as the number is small
assert(Post.connection.column_calls_by_table['comments'] < 3)
end

def test_polymorphic_type_condition def test_polymorphic_type_condition
post = Post.find(posts(:thinking).id, :include => :taggings) post = Post.find(posts(:thinking).id, :include => :taggings)
assert post.taggings.include?(taggings(:thinking_general)) assert post.taggings.include?(taggings(:thinking_general))
Expand Down
5 changes: 4 additions & 1 deletion activerecord/test/cases/helper.rb
Expand Up @@ -39,11 +39,14 @@ def execute_with_query_record(sql, name = nil, &block)
end end


ActiveRecord::Base.connection.class.class_eval { ActiveRecord::Base.connection.class.class_eval {
attr_accessor :column_calls attr_accessor :column_calls, :column_calls_by_table


def columns_with_calls(*args) def columns_with_calls(*args)
@column_calls ||= 0 @column_calls ||= 0
@column_calls_by_table ||= Hash.new {|h,table| h[table] = 0}

@column_calls += 1 @column_calls += 1
@column_calls_by_table[args.first.to_s] += 1
columns_without_calls(*args) columns_without_calls(*args)
end end


Expand Down
1 change: 1 addition & 0 deletions activerecord/test/models/post.rb
Expand Up @@ -44,6 +44,7 @@ def find_most_recent
:conditions => proc { ["#{"#{aliased_table_name}." rescue ""}body = ?", 'Thank you for the welcome'] } :conditions => proc { ["#{"#{aliased_table_name}." rescue ""}body = ?", 'Thank you for the welcome'] }
has_many :comments_with_deprecated_interpolated_conditions, :class_name => 'Comment', has_many :comments_with_deprecated_interpolated_conditions, :class_name => 'Comment',
:conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome'] :conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome']
has_one :first_post_comment, :class_name => 'Comment', :conditions => {:body => 'First Post!'}


has_one :very_special_comment has_one :very_special_comment
has_one :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post has_one :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post
Expand Down

0 comments on commit d6dbd54

Please sign in to comment.