Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #5961 (closed enhancement: untested)

Opened 2 years ago

Last modified 1 year ago

[PATCH] Only update attributes that you want to update.

Reported by: cpunion@gmail.com Assigned to: David
Priority: low Milestone: 1.x
Component: ActiveRecord Version: 1.1.6
Severity: normal Keywords:
Cc:

Description

Patch:

module ActiveRecord
  class Base
    def update_attribute(name, value)
      update_attributes(name => value)
    end

    def update_attributes(new_attributes)
      return if new_attributes.nil?
      attributes = new_attributes.dup
      attributes.stringify_keys!
      self.attributes = attributes
      update(attributes)
    end

    private
      def update(attrs = nil)
        connection.update(
          "UPDATE #{self.class.table_name} " +
          "SET #{quoted_comma_pair_list(connection, attributes_with_quotes(false, attrs))} " +
          "WHERE #{self.class.primary_key} = #{quote(id)}",
          "#{self.class.name} Update"
        )
       
        return true
      end

      def attributes_with_quotes(include_primary_key = true, attrs = nil)
        (attrs || attributes).inject({}) do |quoted, (name, value)|
          if column = column_for_attribute(name)
            quoted[name] = quote(value, column) unless !include_primary_key && column.primary
          end
          quoted
        end
      end
  end
end 


module ActiveRecord
  module ValidationsFix
     def  self.append_features(base)  #  :nodoc:
      super
      base.class_eval do
        alias_method :update_attributes_without_validation, :update_attributes
        alias_method :update_attributes, :update_attributes_with_validation
      end
    end

     def  update_attributes_with_validation(new_attributes)
       return   if  new_attributes.nil?
      attributes = new_attributes.dup
      attributes.stringify_keys!
      self.attributes  =  attributes

       if  valid?
        update_attributes_without_validation(attributes)
       else
         return  false
      end
    end
  end
end

ActiveRecord::Base.class_eval do
  include ActiveRecord::ValidationsFix
end 

Test schema:

create table posts(
	id	serial not null primary key,
	title	varchar(255),
	body	text,
	status	integer not null default 0
);
insert into posts(title, body) values('Hello, World!', 'Long Text...');

Test code:

Post.find(:first).update_attribute(:status, 1)

Without this patch, it generate query string:

UPDATE posts SET "title" = 'Hello, World!', "status" = 1, "body" = 'Long Text...' WHERE id = 1

With this patch:

UPDATE posts SET "status" = 2 WHERE id = 1

Update two attributes:

Post.find(:first).update_attributes(:title => 'Aha', :status => 2)

Query string:

UPDATE posts SET "title" = 'Aha', "status" = 2 WHERE id = 1

Change History

08/30/06 10:02:35 changed by anonymous

  • priority changed from high to low.
  • type changed from defect to enhancement.
  • severity changed from major to normal.

(in reply to: ↑ description ; follow-up: ↓ 4 ) 09/20/06 01:53:47 changed by jaylev

I think this patch, or something like it, is much-needed in Rails; right now, update_attribute is just anti-sugar for attribute=, and doesn't take advantage of the underlying database's concurrency support.

But one thing to think about: how should lock_version behave in such a scenario? If we ignore it (as this patch does), then we're not protected against simultaneous updates of the same field. But if we obey it, we lose the opportunity to handle simultaneous updates of different fields without raising exceptions.

Ah, locking.

01/14/07 20:49:05 changed by dcmanges

  • status changed from new to closed.
  • resolution set to untested.

Can you please provide this patch as an svn diff with unit tests?

(in reply to: ↑ 2 ) 01/14/07 20:53:47 changed by eventualbuddha

Replying to jaylev:

But one thing to think about: how should lock_version behave in such a scenario? If we ignore it (as this patch does), then we're not protected against simultaneous updates of the same field. But if we obey it, we lose the opportunity to handle simultaneous updates of different fields without raising exceptions.

I think that if the model uses locking, then we just handle it as it currently does. The reason for this is basically to maintain consistency.

02/05/07 11:25:49 changed by matrix9180

It seems like this would also eliminate the counter cache issues [5050].