Skip to content

Commit

Permalink
Never attempt to write virtual attributes to the database
Browse files Browse the repository at this point in the history
Currently the place where we limit what gets sent to the database is in
the implementation for `partial_writes`. We should also be restricting
it to column names when partial writes are turned off.

Note that we're using `&` instead of just defaulting to
`self.class.column_names`, as the instance version of `attribute_names`
does not include attributes which are uninitialized (were not included
in the select clause)
  • Loading branch information
sgrif committed Feb 26, 2018
1 parent ef3d695 commit 508742b
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
4 changes: 3 additions & 1 deletion activerecord/lib/active_record/persistence.rb
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,8 @@ def create_or_update(*args, &block)

# Updates the associated record with values matching those of the instance attributes.
# Returns the number of affected rows.
def _update_record(attribute_names = self.attribute_names)
def _update_record(attribute_names = self.column_names)
attribute_names &= self.class.column_names
attributes_values = arel_attributes_with_values_for_update(attribute_names)
if attributes_values.empty?
rows_affected = 0
Expand All @@ -719,6 +720,7 @@ def _update_record(attribute_names = self.attribute_names)
# Creates a record with values matching those of the instance attributes
# and returns its id.
def _create_record(attribute_names = self.attribute_names)
attribute_names &= self.class.column_names
attributes_values = arel_attributes_with_values_for_create(attribute_names)

new_id = self.class._insert_record(attributes_values)
Expand Down
19 changes: 19 additions & 0 deletions activerecord/test/cases/dirty_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,25 @@ def pirate.catchphrase
assert record.save
end

test "virtual attributes are not written with partial_writes off" do
original_partial_writes = ActiveRecord::Base.partial_writes
begin
ActiveRecord::Base.partial_writes = false
klass = Class.new(ActiveRecord::Base) do
self.table_name = "people"
attribute :non_persisted_attribute, :string
end

record = klass.new(first_name: "Sean")
record.non_persisted_attribute_will_change!

assert_predicate record, :non_persisted_attribute_changed?
assert record.save
ensure
ActiveRecord::Base.partial_writes = original_partial_writes
end
end

test "mutating and then assigning doesn't remove the change" do
pirate = Pirate.create!(catchphrase: "arrrr")
pirate.catchphrase << " matey!"
Expand Down

0 comments on commit 508742b

Please sign in to comment.