Skip to content

Commit

Permalink
Prefer _update_record than update_all for updating a record
Browse files Browse the repository at this point in the history
  • Loading branch information
kamipo committed Mar 5, 2018
1 parent 4331478 commit 3798435
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 28 deletions.
7 changes: 2 additions & 5 deletions activerecord/lib/active_record/locking/optimistic.rb
Expand Up @@ -91,13 +91,10 @@ def _update_record(attribute_names = self.attribute_names)

attribute_names.push(lock_col)

relation = self.class.unscoped

affected_rows = relation.where(
affected_rows = self.class._update_record(
attributes_with_values_for_update(attribute_names),
self.class.primary_key => id_in_database,
lock_col => previous_lock_value
).update_all(
attributes_with_values_for_update(attribute_names)
)

unless affected_rows == 1
Expand Down
56 changes: 33 additions & 23 deletions activerecord/lib/active_record/persistence.rb
Expand Up @@ -187,10 +187,11 @@ def _insert_record(values) # :nodoc:
connection.insert(im, "#{self} Create", primary_key || false, primary_key_value)
end

def _update_record(values, id) # :nodoc:
bind = predicate_builder.build_bind_attribute(primary_key, id)
def _update_record(values, constraints) # :nodoc:
constraints = _substitute_values(constraints).map { |attr, bind| attr.eq(bind) }

um = arel_table.where(
arel_attribute(primary_key).eq(bind)
constraints.reduce(&:and)
).compile_update(_substitute_values(values), primary_key)

connection.update(um, "#{self} Update")
Expand Down Expand Up @@ -463,13 +464,16 @@ def update_columns(attributes)
verify_readonly_attribute(key.to_s)
end

updated_count = _relation_for_itself.update_all(attributes)
affected_rows = self.class._update_record(
attributes,
self.class.primary_key => id_in_database
)

attributes.each do |k, v|
write_attribute_without_type_cast(k, v)
end

updated_count == 1
affected_rows == 1
end

# Initializes +attribute+ to zero if +nil+ and adds the value passed as +by+ (default is 1).
Expand Down Expand Up @@ -643,34 +647,37 @@ def touch(*names, time: nil)
end

time ||= current_time_from_proper_timezone
attributes = timestamp_attributes_for_update_in_model
attributes.concat(names)

unless attributes.empty?
changes = {}
attribute_names = timestamp_attributes_for_update_in_model
attribute_names.concat(names)

attributes.each do |column|
column = column.to_s
changes[column] = write_attribute(column, time)
unless attribute_names.empty?
attribute_names.each do |attr_name|
write_attribute(attr_name, time)
end

scope = _relation_for_itself
constraints = { self.class.primary_key => id_in_database }

if locking_enabled?
locking_column = self.class.locking_column
scope = scope.where(locking_column => read_attribute_before_type_cast(locking_column))
changes[locking_column] = increment_lock
constraints[locking_column] = read_attribute_before_type_cast(locking_column)
attribute_names << locking_column
increment_lock
end

clear_attribute_changes(changes.keys)
result = scope.update_all(changes) == 1
clear_attribute_changes(attribute_names)

affected_rows = self.class._update_record(
attributes_with_values_for_update(attribute_names),
constraints
)

result = affected_rows == 1

if !result && locking_enabled?
raise ActiveRecord::StaleObjectError.new(self, "touch")
end

@_trigger_update_callback = result
result
else
true
end
Expand Down Expand Up @@ -707,16 +714,19 @@ def _update_record(attribute_names = self.attribute_names)
attribute_names &= self.class.column_names
attributes_values = attributes_with_values_for_update(attribute_names)
if attributes_values.empty?
rows_affected = 0
affected_rows = 0
@_trigger_update_callback = true
else
rows_affected = self.class._update_record(attributes_values, id_in_database)
@_trigger_update_callback = rows_affected > 0
affected_rows = self.class._update_record(
attributes_values,
self.class.primary_key => id_in_database
)
@_trigger_update_callback = affected_rows == 1
end

yield(self) if block_given?

rows_affected
affected_rows
end

# Creates a record with values matching those of the instance attributes
Expand Down

0 comments on commit 3798435

Please sign in to comment.