From 37984353f272efb12cb1d008cb231a1f1e56524a Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 4 Mar 2018 01:56:49 +0900 Subject: [PATCH] Prefer `_update_record` than `update_all` for updating a record --- .../lib/active_record/locking/optimistic.rb | 7 +-- activerecord/lib/active_record/persistence.rb | 56 +++++++++++-------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index cae7a66fcc2f8..c2f740a1e0082 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -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 diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 28308da96dcc1..610f11b66c964 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -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") @@ -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). @@ -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 @@ -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