update_all can take a Hash argument. sanitize_sql splits into two methods for conditions and assignment since NULL values and delimiters are handled differently. References #6583, closes #7365.

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

3 files changed, 53 insertions, 16 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 85b5262994..b6994bbf62 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* update_all can take a Hash argument. sanitize_sql splits into two methods for conditions and assignment since NULL values and delimiters are handled differently.  #6583, #7365 [sandofsky, Assaf]
+
 * MySQL: SET SQL_AUTO_IS_NULL=0 so 'where id is null' doesn't select the last inserted id.  #6778 [Jonathan Viney, timc]
 
 * Use Date#to_s(:db) for quoted dates.  #7411 [Michael Schoen]
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 0d8daecb69..af3d1997c5 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -491,7 +491,7 @@ module ActiveRecord #:nodoc:
       # A subset of the records can be selected by specifying +conditions+. Example:
       #   Billing.update_all "category = 'authorized', approved = 1", "author = 'David'"
       def update_all(updates, conditions = nil)
-        sql  = "UPDATE #{table_name} SET #{sanitize_sql(updates)} "
+        sql  = "UPDATE #{table_name} SET #{sanitize_sql_for_assignment(updates)} "
         add_conditions!(sql, conditions, scope(:find))
         connection.update(sql, "#{name} Update")
       end
@@ -1383,32 +1383,54 @@ module ActiveRecord #:nodoc:
         end
 
         # Accepts an array, hash, or string of sql conditions and sanitizes
-        # them into a valid SQL fragment.
+        # them into a valid SQL fragment for a WHERE clause.
         #   ["name='%s' and group_id='%s'", "foo'bar", 4]  returns  "name='foo''bar' and group_id='4'"
         #   { :name => "foo'bar", :group_id => 4 }  returns "name='foo''bar' and group_id='4'"
         #   "name='foo''bar' and group_id='4'" returns "name='foo''bar' and group_id='4'"
-        def sanitize_sql(condition)
+        def sanitize_sql_for_conditions(condition)
           case condition
             when Array; sanitize_sql_array(condition)
-            when Hash;  sanitize_sql_hash(condition)
+            when Hash;  sanitize_sql_hash_for_conditions(condition)
             else        condition
           end
         end
+        alias_method :sanitize_sql, :sanitize_sql_for_conditions
 
-        # Sanitizes a hash of attribute/value pairs into SQL conditions.
+        # Accepts an array, hash, or string of sql conditions and sanitizes
+        # them into a valid SQL fragment for a SET clause.
+        #   { :name => nil, :group_id => 4 }  returns "name = NULL , group_id='4'"
+        def sanitize_sql_for_assignment(assignments)
+          case assignments
+            when Array; sanitize_sql_array(assignments)
+            when Hash;  sanitize_sql_hash_for_assignment(assignments)
+            else        assignments
+          end
+        end
+
+        # Sanitizes a hash of attribute/value pairs into SQL conditions for a WHERE clause.
         #   { :name => "foo'bar", :group_id => 4 }
         #     # => "name='foo''bar' and group_id= 4"
         #   { :status => nil, :group_id => [1,2,3] }
         #     # => "status IS NULL and group_id IN (1,2,3)"
         #   { :age => 13..18 }
         #     # => "age BETWEEN 13 AND 18"
-        def sanitize_sql_hash(attrs)
+        def sanitize_sql_hash_for_conditions(attrs)
           conditions = attrs.map do |attr, value|
             "#{table_name}.#{connection.quote_column_name(attr)} #{attribute_condition(value)}"
           end.join(' AND ')
 
           replace_bind_variables(conditions, expand_range_bind_variables(attrs.values))
         end
+        alias_method :sanitize_sql_hash, :sanitize_sql_hash_for_conditions
+
+        # Sanitizes a hash of attribute/value pairs into SQL conditions for a SET clause.
+        #   { :status => nil, :group_id => 1 }
+        #     # => "status = NULL , group_id = 1"
+        def sanitize_sql_hash_for_assignment(attrs)
+          conditions = attrs.map do |attr, value|
+            "#{connection.quote_column_name(attr)} = #{quote_bound_value(value)}"
+          end.join(', ')
+        end
 
         # Accepts an array of conditions.  The array has each value
         # sanitized and interpolated into the sql statement.
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 95c758f805..8892dcbdc8 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -534,17 +534,30 @@ class BasicsTest < Test::Unit::TestCase
     Topic.decrement_counter("replies_count", 2)
     assert_equal -2, Topic.find(2).replies_count
   end
-  
-  def test_update_all
-    # The ADO library doesn't support the number of affected rows
-    return true if current_adapter?(:SQLServerAdapter)
 
-    assert_equal 2, Topic.update_all("content = 'bulk updated!'")
-    assert_equal "bulk updated!", Topic.find(1).content
-    assert_equal "bulk updated!", Topic.find(2).content
-    assert_equal 2, Topic.update_all(['content = ?', 'bulk updated again!'])
-    assert_equal "bulk updated again!", Topic.find(1).content
-    assert_equal "bulk updated again!", Topic.find(2).content
+  # The ADO library doesn't support the number of affected rows
+  unless current_adapter?(:SQLServerAdapter)
+    def test_update_all
+      assert_equal 2, Topic.update_all("content = 'bulk updated!'")
+      assert_equal "bulk updated!", Topic.find(1).content
+      assert_equal "bulk updated!", Topic.find(2).content
+
+      assert_equal 2, Topic.update_all(['content = ?', 'bulk updated again!'])
+      assert_equal "bulk updated again!", Topic.find(1).content
+      assert_equal "bulk updated again!", Topic.find(2).content
+
+      assert_equal 2, Topic.update_all(['content = ?', nil])
+      assert_nil Topic.find(1).content
+    end
+
+    def test_update_all_with_hash
+      assert_not_nil Topic.find(1).last_read
+      assert_equal 2, Topic.update_all(:content => 'bulk updated with hash!', :last_read => nil)
+      assert_equal "bulk updated with hash!", Topic.find(1).content
+      assert_equal "bulk updated with hash!", Topic.find(2).content
+      assert_nil Topic.find(1).last_read
+      assert_nil Topic.find(2).last_read
+    end
   end
 
   def test_update_many