Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Postgres JSON data type #287

Closed
seanlinsley opened this issue Oct 15, 2013 · 13 comments
Closed

Postgres JSON data type #287

seanlinsley opened this issue Oct 15, 2013 · 13 comments
Assignees
Milestone

Comments

@seanlinsley
Copy link
Member

PostgreSQL has a built-in JSON data type, and Rails 4 has native support for it.

Right now I'm using this hack:

module FakeSerializer
  extend self # makes all instance methods become module methods as well

  def load(object)
    object
  end

  def dump(object)
    object
  end
end

PaperTrail.serializer = FakeSerializer

It seems like it could be fairly simple to integrate support into paper_trail

if PaperTrail::Version.columns_hash['object'].type == :json
  # don't use a serializer
else
  # do
end
@ghost ghost assigned batter Oct 16, 2013
@batter
Copy link
Collaborator

batter commented Oct 16, 2013

@daxter - Do you know if there is some documentation available for the HStore support that Rails 4 offers?

I agree that it makes sense to offer this functionality, but I think the solution is not as easy as you suggest. It will need to detect whether the database being used by ActiveRecord is PostgreSQL, and whether the version of ActiveRecord is greater than or equal to 4 or, if not, whether the activerecord-postgres-hstore gem is available. I'll take a crack at it though.

@seanlinsley
Copy link
Member Author

Postgres offers both HStore and JSON data types. HStore is a key-value store for strings, while the JSON data type is fully qualified JSON with numbers, arrays, and nested objects.

There's no need to check for PostgreSQL itself. If Rails is reporting the data type of the column as :json, then you're good. Rails 3 won't report JSON, and non-postgres databases won't report JSON.

@batter
Copy link
Collaborator

batter commented Oct 16, 2013

Ok, then I think it makes sense to just offer support for inserting into columns of type :json and :hstore for ActiveRecord 4 without a serializer by default out of the box, and forget about trying to support the activerecord-postgres-hstore gem. I'll give it a shot, cheers.

@seanlinsley
Copy link
Member Author

It'd probably be best not to try supporting HStore. For example, let's say I have a model that has Postgres Array for one column, and I try using paper_trail with that model. How would HStore handle that? In comparison, the JSON data type supports arrays.

@batter
Copy link
Collaborator

batter commented Oct 16, 2013

@daxter - What is the expected behavior when we are inserting an attribute into the object or object_changes that is a serialized attribute? Should that also not get serialized, or should they still get serialized? (The behavior from the workaround you have in place in your app would result in them still getting serialized prior to insertion in the database).

Also, agreed about HStore after reading some more about the HStore column in PostgreSQL, not sure it can be used since the hashes only support values of type String, so there would need to be a lot of customization for it to work.

@seanlinsley
Copy link
Member Author

What is the expected behavior when we are inserting an attribute into the object or object_changes that is a serialized attribute? Should that also not get serialized, or should they still get serialized?

There shouldn't be any need to serialize it, because Rails will do so automatically.

(The behavior from the workaround you have in place in your app would result in them still getting serialized prior to insertion in the database).

I haven't delved too far into the paper_trail code; where exactly is this happening?

Also, after reading some more about the hstore column in PostgreSQL, not sure it can be used since the hashes only support values of type String, so there would need to be a lot of customization for it to work.

That's exactly my point; HStore probably shouldn't be supported, instead we should focus on the JSON data type.

@batter
Copy link
Collaborator

batter commented Oct 16, 2013

@daxter - So I think I've come up with a successful implementation. The only issue seems to be that datetime issues such as the updated_at and created_at get caste to a String before being stored in the JSON column. Not an issue for the reify method, as ActiveRecord will caste those back to ActiveSupport::TimeWithZone objects in Ruby automatically, but the object_changes column / changeset method doesn't seem to consistently pull them back out of the database as ActiveSupport::TimeWithZone objects. Sometimes they do and sometimes they don't in my local testing. Kind of odd. Don't think that's a major concern, but was wondering if you had experience dealing with that type of thing and had any advice on how to handle it?

@seanlinsley
Copy link
Member Author

No, I'm not sure how to fix that. Could you create a PR so I could see your changes and try them out?

@batter
Copy link
Collaborator

batter commented Oct 16, 2013

Just pushed a branch, json_column_support. Try it out and let me know what you think.

@seanlinsley
Copy link
Member Author

Cool, thanks.

Looking at it, maybe it would be better to create a proxy serializer like I did above. The benefit is that there's less imbedded knowledge interspersed in the code. Instead, it just uses whichever serializer is available.

@batter
Copy link
Collaborator

batter commented Oct 16, 2013

Actually probably a good idea. That being said, the implementation works as you expected, right?

@seanlinsley
Copy link
Member Author

I haven't actually tried using paper_trail that much thus far. That comes tonight :)

@batter
Copy link
Collaborator

batter commented Oct 18, 2013

@daxter - Cleaned up the code on the json_column_support branch. I think it's ready to be merged into the master. I tested it with PostgreSQL and it seemed to work well. Did you give it a shot to see if it worked for your use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants