Anonymous avatar Anonymous committed d6dbd54

Added back the use of the Reflection module's cached sanitized_conditions 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.

Comments (0)

Files changed (5)

activerecord/lib/active_record/associations/association_proxy.rb

       # Returns the SQL string that corresponds to the <tt>:conditions</tt>
       # option of the macro, if given, or +nil+ otherwise.
       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
       alias :sql_conditions :conditions
 
           @reflection.options[:dependent]
         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)
           @owner.send(:interpolate_and_sanitize_sql, sql, record, sanitize_klass)
         end

activerecord/lib/active_record/base.rb

 
       def interpolate_and_sanitize_sql(sql, record = nil, sanitize_klass = self.class)
         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 =~ /\#\{.*\}/
           ActiveSupport::Deprecation.warn(
             'String-based interpolation of association conditions is deprecated. Please use a ' \
             'should be changed to has_many :older_friends, :conditions => proc { "age > #{age}" }.'
           )
           instance_eval("%@#{sanitized.gsub('@', '\@')}@", __FILE__, __LINE__)
-        elsif sql.respond_to?(:to_proc)
-          sanitize_klass.send(:sanitize_sql, instance_exec(record, &sql))
+        elsif sanitized.respond_to?(:to_proc)
+          sanitize_klass.send(:sanitize_sql, instance_exec(record, &sanitized))
         else
           sanitized
         end

activerecord/test/cases/associations/eager_test.rb

     assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
   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
     post = Post.find(posts(:thinking).id, :include => :taggings)
     assert post.taggings.include?(taggings(:thinking_general))

activerecord/test/cases/helper.rb

 end
 
 ActiveRecord::Base.connection.class.class_eval {
-  attr_accessor :column_calls
+  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
 

activerecord/test/models/post.rb

       :conditions => proc { ["#{"#{aliased_table_name}." rescue ""}body = ?", 'Thank you for the welcome'] }
   has_many :comments_with_deprecated_interpolated_conditions, :class_name => 'Comment',
       :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_with_post, :class_name => "VerySpecialComment", :include => :post
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.