partial_updates clobbers serialized attributes

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 Dirty checking is broken for string join for more info.

Prior to dirty tracking changes made to serialized objects were saved. Now they are not. This is the way things are going to stay?

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.

It would be nice, but this would make the behaviour inconsistent with the (odd but expected) behaviour of other attributes (see #608 Dirty checking is broken for string join).

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.

yuck. Well thanks for the heads up on #608 Dirty checking is broken for string join and the patch Tom.

(from [decc9730959de85b4c4873986bcd33e12fa1985d]) Serialized attributes will now always be saved even with partial_updates turned on.

Signed-off-by: Michael Koziarski michael@koziarski.com [#788 partial_updates clobbers serialized attributes state:committed] http://github.com/rails/rails/co...

Normally I wouldn't like a special case like this, but as 99.9% of the time you're modifying serialized attributes in place.

Thanks for the commit Koz!

Thanks Koz.

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.

We're considering a minor release shortly and this will be included in it.

May I hug you ?

Thank god/Tom/Koz for that. I've just spent a whole day tracking down a bug that was due to this misbehaviour.

Sad this isn't in 2.0.4.

John, partial updates were first introduced in 2.1 so backporting this patch wouldn't make sense for 2.0.x.

Thanks Jeremy. That was awfully stupid of me.

John, hey no worries :)

I'm not sure why this is marked as committed only, rather than resolved, though. Koz?

I've been using committed for all my patches I apply. committed isn't finished?

Oops sorry it is. Not it the changelog I guess.

It is in 2.1.1 isn't it?

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...

Not presently but if you'd like to submit a patch for it we could apply it

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.