#788 partial_updates clobbers serialized attributes - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#788 ✓committed

partial_updates clobbers serialized attributes

Reported by John Wulff | August 9th, 2008 @ 03:33 AM | in 2.x


class Node < ActiveRecord::Base
  serialize :data
end
>> n = Node.create! :data => { :a => 1 }
=> #<Node id: 417950, data: {:a=>1}>
>> n.id
=> 417950
>> m = Node.find 417950
=> #<Node id: 417950, data: {:a=>1}>
>> m.data[:b] = 2
=> 2
>> m
=> #<Node id: 417950, data: {:b=>2, :a=>1}>
>> m.changed?
=> false
>> m.save!
=> true
>> m = Node.find 417950
=> #<Node id: 417950, data: {:a=>1}>

Changes to the hash are never saved and that makes me really, really sad.

Comments and changes to this ticket

You flagged this item as spam.
Ryan Bates August 9th, 2008 @ 04:36 AM
This is a known behavior of dirty tracking. It can't detect changes made directly on an attribute. You have to either go through the accessor method or warn dirty tracking you're changing the attribute.
```
m.data = {:b=>2, :a=>1}

# or

m.data_will_change!
m.data[:b] = 2
```
See ticket #608 for more info.
You flagged this item as spam.
John Wulff August 9th, 2008 @ 05:11 AM
Prior to dirty tracking changes made to serialized objects were saved. Now they are not. This is the way things are going to stay?
You flagged this item as spam.
Tom Lea August 11th, 2008 @ 06:20 PM
- Tag changed from “activerecord, bug” to “2.1, activerecord, bug, patch”
This looks like a bug to me... with partial_updates update will only save attributes that are dirty, so serialized attributes are skipped.

Trivial quick patch attached to resolve this.
- should_save_serialized_attributes_even_with_partial_updates.diff 1.7 KB
You flagged this item as spam.
Doug Cole August 11th, 2008 @ 09:23 PM
I apologize for criticizing without offering up my own patch, but it seems like this patch lacking a few things. Why not rewrite changed? for serialized attributes that checks the serialization against the old value and stores the results. Then run changed? against all serialized attributes at the beginning of update_with_dirty. It'll be slower, but it will then behave as expected rather than always assuming serialized attributes are changed.

As an aside if this was a known behavior of rails 2.1, it seems like it should have been better publicized. I couldn't find any mention of it even though it breaks nearly every rails app that uses serialized attributes.
You flagged this item as spam.
Tom Lea August 11th, 2008 @ 10:27 PM
It would be nice, but this would make the behaviour inconsistent with the (odd but expected) behaviour of other attributes (see #608).

The patch will not fix any issues with AR#changed? & family. Just a bug with partial_updates.

If we wished to have a comparison with previous values, we would have to stash the values of all attributes at record load. This would be a very different approach to what is currently there.

But I digress...

The patch makes things save as expected in 2.0, preventing 'mystery' data loss, it does not change the AR#changed? 2.1 features.
You flagged this item as spam.
Doug Cole August 11th, 2008 @ 10:42 PM
yuck. Well thanks for the heads up on #608 and the patch Tom.
You flagged this item as spam.
Tom Lea August 12th, 2008 @ 01:57 PM
- Title changed from “Saving Only "changed" Attributes Hurts Serialization” to “partial_updates clobbers serialized attributes”
Repository August 12th, 2008 @ 05:17 PM
- State changed from “new” to “committed”
(from [992fda16ed662f028700d63a8dcbd1837f1d58ab]) Serialized attributes will now always be saved even with partial_updates turned on.

Signed-off-by: Michael Koziarski michael@koziarski.com [#788 state:committed] http://github.com/rails/rails/co...
Repository August 12th, 2008 @ 05:19 PM
(from [decc9730959de85b4c4873986bcd33e12fa1985d]) Serialized attributes will now always be saved even with partial_updates turned on.

Signed-off-by: Michael Koziarski michael@koziarski.com [#788 state:committed] http://github.com/rails/rails/co...
Michael Koziarski August 12th, 2008 @ 05:20 PM
Normally I wouldn't like a special case like this, but as 99.9% of the time you're modifying serialized attributes in place.
You flagged this item as spam.
Tom Lea August 12th, 2008 @ 05:39 PM
Thanks for the commit Koz!
You flagged this item as spam.
John Wulff August 13th, 2008 @ 01:02 AM
Thanks Koz.
You flagged this item as spam.
Jérôme August 23rd, 2008 @ 11:00 PM
Wow, this is really a critical bug as this buggy behavior breaks applications when the Dirty module was introduced to rails. This patch deserves a minor release and Tom some national honors.
Michael Koziarski August 24th, 2008 @ 01:35 PM
We're considering a minor release shortly and this will be included in it.
You flagged this item as spam.
Jérôme August 24th, 2008 @ 01:46 PM
May I hug you ?
You flagged this item as spam.
Steve Purcell August 27th, 2008 @ 05:02 PM
Thank god/Tom/Koz for that. I've just spent a whole day tracking down a bug that was due to this misbehaviour.
You flagged this item as spam.
John Wulff September 3rd, 2008 @ 07:23 PM
Sad this isn't in 2.0.4.
Jeremy Kemper September 3rd, 2008 @ 10:13 PM
John, partial updates were first introduced in 2.1 so backporting this patch wouldn't make sense for 2.0.x.
You flagged this item as spam.
John Wulff September 3rd, 2008 @ 10:38 PM
Thanks Jeremy. That was awfully stupid of me.
Jeremy Kemper September 3rd, 2008 @ 10:41 PM
John, hey no worries :)

I'm not sure why this is marked as committed only, rather than resolved, though. Koz?
Michael Koziarski September 4th, 2008 @ 08:32 AM
I've been using committed for all my patches I apply. committed isn't finished?
You flagged this item as spam.
Jérôme September 23rd, 2008 @ 10:28 AM
- Tag changed from “2.1, activerecord, bug, patch” to “2.1, activerecord, bug, patch”
Then sad this isn't in rails 2.1.1 :(
You flagged this item as spam.
Jérôme September 23rd, 2008 @ 10:31 AM
Oops sorry it is. Not it the changelog I guess.
Michael Koziarski September 23rd, 2008 @ 10:33 AM
It is in 2.1.1 isn't it?
You flagged this item as spam.
Fjan October 4th, 2008 @ 04:48 PM
Yes, it is in 2.1.1. I understand the reasoning for marking serialized objects as dirty each save, to be consistent with earlier behaviour. But is there any way to get the old behaviour back? I have a couple of large serialized objects that now get serialized and saved every time...
Michael Koziarski October 4th, 2008 @ 04:53 PM
Not presently but if you'd like to submit a patch for it we could apply it
You flagged this item as spam.
Fjan October 4th, 2008 @ 07:17 PM
Ok, I looked at the code and it seems relatively straightforward to add support for something like:

ActiveRecord::Base.partial_updates = false | true | :serialized_also

Should be about 3 lines of code which seems a pretty worthwhile effort to avoid expensive serializing. But I'm afraid I'm not very experienced (I've yet to submit a patch for anything) so I'm going to defer that until I have a rainy afternoon to figure out that whole git and diff thing. Unless someone else wants to take up the glove a monkey patch will do for now.
You flagged this item as spam.
Fjan August 9th, 2009 @ 04:13 PM
- Tag changed from “2.1, activerecord, bug, patch” to “2.1, activerecord, bug, patch”
Unfortunately, this patch has caused bug 2397: it will now even save serialized attributes if they are not in memory, setting them to null. (Let's continue the discussion there)