This project is archived and is in readonly mode.

#788 ✓committed
John Wulff

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

  • Ryan Bates

    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.

  • John Wulff

    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?

  • Tom Lea

    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.

  • Doug Cole

    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.

  • Tom Lea

    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.

  • Doug Cole

    Doug Cole August 11th, 2008 @ 10:42 PM

    yuck. Well thanks for the heads up on #608 and the patch Tom.

  • Tom Lea

    Tom Lea August 12th, 2008 @ 01:57 PM

    • Title changed from “Saving Only "changed" Attributes Hurts Serialization” to “partial_updates clobbers serialized attributes”
  • Repository

    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

    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

    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.

  • Tom Lea

    Tom Lea August 12th, 2008 @ 05:39 PM

    Thanks for the commit Koz!

  • John Wulff
  • Jérôme

    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

    Michael Koziarski August 24th, 2008 @ 01:35 PM

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

  • Jérôme
  • Steve Purcell

    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.

  • John Wulff

    John Wulff September 3rd, 2008 @ 07:23 PM

    Sad this isn't in 2.0.4.

  • Jeremy Kemper

    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.

  • John Wulff

    John Wulff September 3rd, 2008 @ 10:38 PM

    Thanks Jeremy. That was awfully stupid of me.

  • Jeremy Kemper

    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

    Michael Koziarski September 4th, 2008 @ 08:32 AM

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

  • Jérôme

    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 :(

  • Jérôme

    Jérôme September 23rd, 2008 @ 10:31 AM

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

  • Michael Koziarski
  • Fjan

    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

    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

  • Fjan

    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.

  • Fjan

    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)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages