Postgres hstore conflicts with serialization #14411

Closed
leods92 opened this Issue Mar 17, 2014 · 10 comments

Projects

None yet

7 participants

@leods92
leods92 commented Mar 17, 2014

Think of the following object:

class User < ActiveRecord::Base
  class Options
    class Serializer
      def self.load(hash)
        Options.new(hash)
      end

      def self.dump(object)
        object.to_hash
      end
    end

    attr_accessor :background

    def to_hash
      { background: background }
    end

    private

    def initialize(attributes)
      self.background = attributes[:background]
    end
  end

  serialize :options, Options::Serializer
end

The options column is a Postgre hstore one supported since Rails 4.0.0.

When object gets initialized of fetched from database, User::Options::Serializer.load receives a hash (Rails typecasted pg's hstore into a hash).

However, when duplicating an object, User::Options::Serializer.load gets the raw Postgres hstore; which is unexpected.

I've found a workaround which is to check the type of input and if it's a string convert it to a hash using #string_to_hstore. But it's not the right thing to do, is it?

I've created an app and a test to better demonstrate this situation:
https://github.com/leods92/hstore_dup_bug

Maybe this was by design but I don't think so since Rails tends to be very standard and to meet one's expectations.
One might argue that since I'm using a Serializer, input should be unserialized. Okay, great, but in this case Serializer.load should always get hstore raw string.
I don't really agree with that since a Serializer should only handle its own serialization not typecasting.

Maybe the problem is in active_record/attribute_methods/serialization.rb on #read_attribute_before_type_cast.
I feel that I'm very close to understand the whole process but since I've already spent hours on this I decided to post this and let somebody more experienced give his thoughts on this.

Some issues that might be related to this problem:
#5797
#4837

@jbourassa
Contributor

Hi @leods92,

In an attempt to fix issues on OpenSource projects I love, I've worked on this today and I found some more information. It's not yet a fix, but maybe it'll help:

  • The field is lazy-unserialized and unserializing the field updates the attribute_before_type_cast value (not sure why nor how):
> u = User.find(2222)

> u.send(:read_attribute_before_type_cast, :options)
=> #<struct ActiveRecord::AttributeMethods::Serialization::Attribute
 coder=User::Options::Serializer,
 value="\"background\"=>\"#FF0000\""

> u.options

> u.send(:read_attribute_before_type_cast, :options)
=> #<struct ActiveRecord::AttributeMethods::Serialization::Attribute
 coder=User::Options::Serializer,
 value=#<User::Options:0x007ffb1db7aff0 @background="#FF0000">,
 state=:unserialized>
  • dup reads the attribute_before_type_cast and sends it to the deserializer. This means that the type can change depending if the property (options here) has been called or not.

I hope that helps somehow.

@senny
Member
senny commented Mar 26, 2014

I can confirm the issue (https://gist.github.com/senny/9780268)

@chris-teague

This doesn't seem unique to hstore, swapping tags to json datatype on your patch senny showed the same issue & symptoms.

@senny
Member
senny commented Mar 28, 2014

@chris-teague that's right. Every datatype that has customized casting behavior will probably show similar behavior.

@leods92
leods92 commented Apr 4, 2014

@gjaldon I've fixed it in my application by changing the serializer and converting hstore to hash if input is a string. Anyway, this should be fixed in Rails.

When it comes to #clone and #dup, there are some differences on their behaviour.
From the docs:
Be warned that your attributes are not copied. That means that modifying attributes of the clone will modify the original, since they will both point to the same attributes hash.

@gjaldon
gjaldon commented Apr 4, 2014

Had actually deleted my question when I found the answer. I appreciate the response though, @leods92!

Feel free to check #14595. It has some failures on Travis which don't seem to be related with the changes I've done. Saw other recent PR's that are also 'suffering' from the same error that has to do with 'psych'.

@robin850 robin850 added the attached PR label Apr 5, 2014
@chris-teague

👍 that PR fixed my issues

@senny
Member
senny commented May 30, 2014

/cc @sgrif

@sgrif
Member
sgrif commented May 30, 2014

Working on rewriting serialized attributes at the moment, encountered this and some related bugs yesterday. There's several inconsistencies in the type casting behavior which I'm looking to address. One of those is figuring out what _before_type_cast means for serialized types. I do not think the attached PR is a good solution here. A more immediate answer would be to read @attributes directly rather than going through read_attribute_before_type_cast during dup, but it should be irrelevant with the refactoring I'm working on, as I'm removing most/all of the special cases for serialized attributes.

@senny senny added a commit that referenced this issue Jun 6, 2014
@senny senny serialized Type should delegate `type_cast_for_write` to underlying Type
This adds a regression test for #14411, which was fixed by #15503.

Closes #14411
Closes #14595
69274ce
@senny senny added a commit that closed this issue Jun 6, 2014
@senny senny serialized Type should delegate `type_cast_for_write` to underlying Type
This adds a regression test for #14411, which was fixed by #15503.

Closes #14411
Closes #14595
69274ce
@senny senny closed this in 69274ce Jun 6, 2014
@senny
Member
senny commented Jun 6, 2014

This was solved by @sgrif and #15503

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment