Skip to content
This repository
Browse code

Added back the use of the Reflection module's cached sanitized_condit…

…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...
commit 03852b6bcf5d26bd22f7422a8fdd4763c0c11e07 1 parent 4e2bacd
Patrick Bacon baconpat authored
6 activerecord/lib/active_record/associations/association_proxy.rb
@@ -102,7 +102,7 @@ def aliased_table_name
102 102 # Returns the SQL string that corresponds to the <tt>:conditions</tt>
103 103 # option of the macro, if given, or +nil+ otherwise.
104 104 def conditions
105   - @conditions ||= @reflection.options[:conditions] && interpolate_and_sanitize_sql(@reflection.options[:conditions])
  105 + @conditions ||= interpolate_sanitized_sql(@reflection.sanitized_conditions) if @reflection.sanitized_conditions
106 106 end
107 107 alias :sql_conditions :conditions
108 108
@@ -161,6 +161,10 @@ def dependent?
161 161 @reflection.options[:dependent]
162 162 end
163 163
  164 + def interpolate_sanitized_sql(sql, record = nil, sanitize_klass = @reflection.klass)
  165 + @owner.send(:interpolate_sanitized_sql, sql, record, sanitize_klass)
  166 + end
  167 +
164 168 def interpolate_and_sanitize_sql(sql, record = nil, sanitize_klass = @reflection.klass)
165 169 @owner.send(:interpolate_and_sanitize_sql, sql, record, sanitize_klass)
166 170 end
7 activerecord/lib/active_record/base.rb
@@ -1733,7 +1733,10 @@ def quote_value(value, column = nil)
1733 1733
1734 1734 def interpolate_and_sanitize_sql(sql, record = nil, sanitize_klass = self.class)
1735 1735 sanitized = sanitize_klass.send(:sanitize_sql, sql)
  1736 + interpolate_sanitized_sql(sanitized, record, sanitize_klass)
  1737 + end
1736 1738
  1739 + def interpolate_sanitized_sql(sanitized, record = nil, sanitize_klass = self.class)
1737 1740 if sanitized =~ /\#\{.*\}/
1738 1741 ActiveSupport::Deprecation.warn(
1739 1742 'String-based interpolation of association conditions is deprecated. Please use a ' \
@@ -1741,8 +1744,8 @@ def interpolate_and_sanitize_sql(sql, record = nil, sanitize_klass = self.class)
1741 1744 'should be changed to has_many :older_friends, :conditions => proc { "age > #{age}" }.'
1742 1745 )
1743 1746 instance_eval("%@#{sanitized.gsub('@', '\@')}@", __FILE__, __LINE__)
1744   - elsif sql.respond_to?(:to_proc)
1745   - sanitize_klass.send(:sanitize_sql, instance_exec(record, &sql))
  1747 + elsif sanitized.respond_to?(:to_proc)
  1748 + sanitize_klass.send(:sanitize_sql, instance_exec(record, &sanitized))
1746 1749 else
1747 1750 sanitized
1748 1751 end
17 activerecord/test/cases/associations/eager_test.rb
@@ -670,6 +670,23 @@ def test_preload_with_deprecated_interpolation
670 670 assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
671 671 end
672 672
  673 + def test_preload_has_one_with_conditions
  674 + # pre-heat our cache
  675 + Post.arel_table.columns
  676 + Comment.columns
  677 +
  678 + Post.connection.column_calls_by_table['comments'] = 0
  679 + Post.includes(:very_special_comment).all.to_a
  680 + assert_equal 0, Post.connection.column_calls_by_table['comments']
  681 +
  682 + Post.connection.column_calls_by_table['comments'] = 0
  683 + Post.includes(:first_post_comment).all.to_a
  684 +
  685 + # Don't care exactly how many column lookup are done,
  686 + # as long as the number is small
  687 + assert(Post.connection.column_calls_by_table['comments'] < 3)
  688 + end
  689 +
673 690 def test_polymorphic_type_condition
674 691 post = Post.find(posts(:thinking).id, :include => :taggings)
675 692 assert post.taggings.include?(taggings(:thinking_general))
5 activerecord/test/cases/helper.rb
@@ -39,11 +39,14 @@ def execute_with_query_record(sql, name = nil, &block)
39 39 end
40 40
41 41 ActiveRecord::Base.connection.class.class_eval {
42   - attr_accessor :column_calls
  42 + attr_accessor :column_calls, :column_calls_by_table
43 43
44 44 def columns_with_calls(*args)
45 45 @column_calls ||= 0
  46 + @column_calls_by_table ||= Hash.new {|h,table| h[table] = 0}
  47 +
46 48 @column_calls += 1
  49 + @column_calls_by_table[args.first.to_s] += 1
47 50 columns_without_calls(*args)
48 51 end
49 52
1  activerecord/test/models/post.rb
@@ -44,6 +44,7 @@ def find_most_recent
44 44 :conditions => proc { ["#{"#{aliased_table_name}." rescue ""}body = ?", 'Thank you for the welcome'] }
45 45 has_many :comments_with_deprecated_interpolated_conditions, :class_name => 'Comment',
46 46 :conditions => ['#{"#{aliased_table_name}." rescue ""}body = ?', 'Thank you for the welcome']
  47 + has_one :first_post_comment, :class_name => 'Comment', :conditions => {:body => 'First Post!'}
47 48
48 49 has_one :very_special_comment
49 50 has_one :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post

0 comments on commit 03852b6

Please sign in to comment.
Something went wrong with that request. Please try again.