This project is archived and is in readonly mode.
serialized attributes clobbered on partial updates
Reported by Fjan | April 2nd, 2009 @ 09:07 AM
This issue caused a serious database corruption for me. If you partially retrieve an object from the database with :select and then save it, any serialized attributes that were NOT in the original select will be set to nil!
This is bound to trip people up because serialized attributes are expensive and are prime targets to leave in the database if you don't need them. And my tests missed it because you don't find out the data is gone until you try to read it back in. As far as I can tell this is malfunctioning of the "dirty" tracking.
Comments and changes to this ticket
-
CancelProfileIsBroken August 6th, 2009 @ 12:55 PM
- Tag set to “bugmash”
-
-
-
Mike Breen August 9th, 2009 @ 04:19 PM
- Assigned user set to “Pratik”
- Tag changed from “bugmash” to “activerecord, bugmash, dirty, serialized”
I've attached a patch that will only save the serialized attribute if it is present in the attributes.
-
Fjan August 9th, 2009 @ 04:25 PM
- Assigned user cleared.
- Tag changed from “activerecord, bugmash, dirty, serialized” to “bugmash”
So the gist of the #788 discussion is that partial updates are now always saved because their modification typically cannot detected. The earlier solution that required attribute_changed! to be called on serialised attributes was deemed to be non-intuitive.
This is a dilemma:
- Always saving them causes this serious bug, but is also wasteful because it saves non-changed attributes that are typically large and expensive to serialize - Not saving them trips people up because they don't realize in place modification doesn't update the dirty tracking -
Fjan August 9th, 2009 @ 04:28 PM
- Assigned user set to “Pratik”
- Tag changed from “bugmash” to “activerecord, bugmash, dirty, serialized”
Oops sorry about that last one: we saved at the same time. Thanks for the elegant solution.
-
Repository August 9th, 2009 @ 04:43 PM
- State changed from “new” to “resolved”
(from [8056c57a94a2cbbbeb662ebe5b0cc6aa0ca1ef00]) Serialized attributes should only be saved with partial_updates when the serialized attribute is present [#2397 state:resolved]
Signed-off-by: Pratik Naik pratiknaik@gmail.com
http://github.com/rails/rails/commit/8056c57a94a2cbbbeb662ebe5b0cc6... -
Repository August 9th, 2009 @ 04:43 PM
(from [7d254b5d74144a1e217125e7be21882ce380a3f8]) Serialized attributes should only be saved with partial_updates when the serialized attribute is present [#2397 state:resolved]
Signed-off-by: Pratik Naik pratiknaik@gmail.com
http://github.com/rails/rails/commit/7d254b5d74144a1e217125e7be2188... -
CancelProfileIsBroken August 9th, 2009 @ 05:13 PM
- Assigned user cleared.
- Tag changed from “activerecord, bugmash, dirty, serialized” to “activerecord, dirty, serialized”
- Milestone cleared.
-
Neil Spring December 22nd, 2009 @ 02:13 PM
Could this patch be changed from:
update_without_dirty(changed | (attributes.keys & self.class.serialized_attributes.keys))
to:
update_without_dirty(changed | (attribute_names & self.class.serialized_attributes.keys))
I believe the two have the same effect. For me, attributes.keys requires reading all attributes to fill in the values. I'm continuing to port old software written when ModelSecurity worked well, and reading all elements of a record is not permitted. For others, going directly to the attribute names should be more efficient.
-
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>
People watching this ticket
CancelProfileIsBroken
José Valim
Mike Breen
Pratik
Attachments
Tags
Referenced by
-
788
partial_updates clobbers serialized attributes
Unfortunately, this patch has caused bug
2397: it will n...
-
2397
serialized attributes clobbered on partial updates
(from [8056c57a94a2cbbbeb662ebe5b0cc6aa0ca1ef00])
Seriali...
-
2397
serialized attributes clobbered on partial updates
(from [7d254b5d74144a1e217125e7be21882ce380a3f8])
Seriali...
-
3678
Small tweak to fix unnecessary duping and loading
Commit 8056c57 fixed #2397, but it introduces a little
in...
-
3678
Small tweak to fix unnecessary duping and loading
This does apply cleanly to master also - but you need to
...
-
4137
Dirty checking on serialize attributes hides changes on concurrent updates
The solution of #788 to save all serialize attributes cre...