Removes the ability for eager loaded conditions to be interpolated, s…

…ince there is no model instance to use as a context for interpolation. #5553 [turnip@turnipspatch.com]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5264 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Removes the ability for eager loaded conditions to be interpolated, since there is no model instance to use as a context for interpolation. #5553 [turnip@turnipspatch.com]
+
 * Added timeout option to SQLite3 configurations to deal more gracefully with SQLite3::BusyException, now the connection can instead retry for x seconds to see if the db clears up before throwing that exception #6126 [wreese@gmail.com]
 
 * Added update_attributes! which uses save! to raise an exception if a validation error prevents saving #6192 [jonathan]

activerecord/lib/active_record/associations.rb

@@ -349,23 +349,17 @@ def clear_association_cache #:nodoc:
     # But that shouldn't fool you to think that you can pull out huge amounts of data with no performance penalty just because you've reduced
     # the number of queries. The database still needs to send all the data to Active Record and it still needs to be processed. So it's no
     # catch-all for performance problems, but it's a great way to cut down on the number of queries in a situation as the one described above.
+    # 
+    # Since the eager loading pulls from multiple tables, you'll have to disambiguate any column references in both conditions and orders. So
+    # :order => "posts.id DESC" will work while :order => "id DESC" will not. Because eager loading generates the SELECT statement too, the
+    # :select option is ignored.
     #
-    # Please note that limited eager loading with has_many and has_and_belongs_to_many associations is not compatible with describing conditions
-    # on these eager tables. This will work:
-    #
-    #   Post.find(:all, :include => :comments, :conditions => "posts.title = 'magic forest'", :limit => 2)
-    #
-    # ...but this will not (and an ArgumentError will be raised):
-    #
-    #   Post.find(:all, :include => :comments, :conditions => "comments.body like 'Normal%'", :limit => 2)
-    #
-    # Also have in mind that since the eager loading is pulling from multiple tables, you'll have to disambiguate any column references
-    # in both conditions and orders. So :order => "posts.id DESC" will work while :order => "id DESC" will not. This may require that
-    # you alter the :order and :conditions on the association definitions themselves.  Because eager loading generates the SELECT statement too,
-    # the :select option is ignored.
-    #
-    # It's currently not possible to use eager loading on multiple associations from the same table. Eager loading will not pull
-    # additional attributes on join tables, so "rich associations" with has_and_belongs_to_many is not a good fit for eager loading.
+    # You can use eager loading on multiple associations from the same table, but you cannot use those associations in orders and conditions
+    # as there is currently not any way to disambiguate them. Eager loading will not pull additional attributes on join tables, so "rich
+    # associations" with has_and_belongs_to_many are not a good fit for eager loading.
+    # 
+    # When eager loaded, conditions are not interpolated. This is because the interpolation of lazy loaded conditions happens within the context
+    # of the object; when eager loading the object does not exist yet.
     # 
     # == Table Aliasing
     #
@@ -1567,7 +1561,7 @@ def association_join
                 aliased_table_name, 
                 reflection.active_record.connection.quote_column_name(reflection.active_record.inheritance_column), 
                 klass.quote_value(klass.name.demodulize)] unless klass.descends_from_active_record?
-              join << "AND #{interpolate_sql(sanitize_sql(reflection.options[:conditions]))} " if reflection.options[:conditions]
+              join << "AND #{sanitize_sql(reflection.options[:conditions])} " if reflection.options[:conditions]
               join
             end
 
@@ -1583,10 +1577,6 @@ def table_alias_for(table_name, table_alias)
               def table_name_and_alias
                 table_alias_for table_name, @aliased_table_name
               end
-
-              def interpolate_sql(sql)
-                instance_eval("%@#{sql.gsub('@', '\@')}@")
-              end
           end
         end
     end

activerecord/lib/active_record/associations/association_proxy.rb

@@ -41,7 +41,7 @@ def aliased_table_name
       end
 
       def conditions
-        @conditions ||= eval("%(#{@reflection.klass.send :sanitize_sql, @reflection.options[:conditions]})") if @reflection.options[:conditions]
+        @conditions ||= interpolate_sql(sanitize_sql(@reflection.options[:conditions])) if @reflection.options[:conditions]
       end
       alias :sql_conditions :conditions
 

activerecord/test/associations/eager_test.rb

@@ -283,7 +283,7 @@ def assert_equal_after_sort(item1, item2, item3 = nil)
     end
     # Test regular association, association with conditions, association with
     # STI, and association with conditions assured not to be true
-    post_types = [:posts, :hello_posts, :special_posts, :nonexistent_posts]
+    post_types = [:posts, :other_posts, :special_posts]
     # test both has_many and has_and_belongs_to_many
     [Author, Category].each do |className|
       d1 = find_all_ordered(className)

activerecord/test/associations_test.rb

@@ -315,6 +315,14 @@ def test_assignment_before_parent_saved
     assert_equal a, firm.account
     assert_equal a, firm.account(true)
   end
+
+  def test_finding_with_interpolated_condition
+    firm = Firm.find(:first)
+    superior = firm.clients.create(:name => 'SuperiorCo')
+    superior.rating = 10
+    superior.save
+    assert_equal 10, firm.clients_with_interpolated_conditions.first.rating
+  end
 
   def test_assignment_before_child_saved
     firm = Firm.find(1)

activerecord/test/fixtures/author.rb

@@ -18,8 +18,7 @@ def testing_proxy_target
   has_many :funky_comments, :through => :posts, :source => :comments
 
   has_many :special_posts,        :class_name => "Post"
-  has_many :hello_posts,          :class_name => "Post", :conditions=>"\#{aliased_table_name}.body = 'hello'"
-  has_many :nonexistent_posts,    :class_name => "Post", :conditions=>"\#{aliased_table_name}.body = 'nonexistent'"
+  has_many :other_posts,          :class_name => "Post"
   has_many :posts_with_callbacks, :class_name => "Post", :before_add => :log_before_adding,
            :after_add     => :log_after_adding, 
            :before_remove => :log_before_removing,

activerecord/test/fixtures/category.rb

@@ -1,8 +1,7 @@
 class Category < ActiveRecord::Base
   has_and_belongs_to_many :posts
   has_and_belongs_to_many :special_posts, :class_name => "Post"
-  has_and_belongs_to_many :hello_posts, :class_name => "Post", :conditions => "\#{aliased_table_name}.body = 'hello'"
-  has_and_belongs_to_many :nonexistent_posts, :class_name => "Post", :conditions=>"\#{aliased_table_name}.body = 'nonexistent'"
+  has_and_belongs_to_many :other_posts, :class_name => "Post"
 
   def self.what_are_you
     'a category...'

activerecord/test/fixtures/company.rb

@@ -22,6 +22,7 @@ class Firm < Company
   has_many :exclusively_dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all
   has_many :limited_clients, :class_name => "Client", :order => "id", :limit => 1
   has_many :clients_like_ms, :conditions => "name = 'Microsoft'", :class_name => "Client", :order => "id"
+  has_many :clients_with_interpolated_conditions, :class_name => "Client", :conditions => 'rating > #{rating}'
   has_many :clients_like_ms_with_hash_conditions, :conditions => { :name => 'Microsoft' }, :class_name => "Client", :order => "id"
   has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}'
   has_many :clients_using_counter_sql, :class_name => "Client",

activerecord/test/reflection_test.rb

@@ -143,8 +143,8 @@ def test_association_reflection_in_modules
   end
 
   def test_reflection_of_all_associations
-    assert_equal 16, Firm.reflect_on_all_associations.size
-    assert_equal 14, Firm.reflect_on_all_associations(:has_many).size
+    assert_equal 17, Firm.reflect_on_all_associations.size
+    assert_equal 15, Firm.reflect_on_all_associations(:has_many).size
     assert_equal 2, Firm.reflect_on_all_associations(:has_one).size
     assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size
   end