AR supporting new JSON data type on PostgreSQL >= 9.2 #7527
Conversation
| end | ||
|
|
||
| def teardown | ||
| @connection.execute 'drop table if exists json_data_type' |
There was a problem hiding this comment.
Do you mean this instead ?
@connection.execute 'drop table' if exists json_data_typeThere was a problem hiding this comment.
Hi!
No, I'm not. json_data_type is the name I'm using for the test table. See line 16 above.
Thanks for your review.
There was a problem hiding this comment.
Sorry, I thought. ^^ BTW, thanks for this pull request, awesome.
|
I'm 👍 for this feature ! :-) |
| else | ||
| string | ||
| end | ||
| end |
There was a problem hiding this comment.
Shouldn't it use ActiveSupport::JSON?
|
Looks good, I've made a few comments. Thanks! |
|
And apparently after 9e0a14f you'll have to rebase and move some code around :) |
|
Oopss! I didn't see this pull request. Sorry |
|
@carlosantoniodasilva Thank you for your suggestions, I rebased from master and did the changes squashing my commits. I ran the tests against postgres 9.1 and 9.2RC1 and everything passed. /cc @rafaelfranca |
| @connection.create_table('json_data_type') do |t| | ||
| t.json 'payload', :default => {} | ||
| end | ||
| end |
There was a problem hiding this comment.
Do you need the transaction block to create only one table?
There was a problem hiding this comment.
No, I don't. I'll remove it.
|
@guedes everything looks great! I just added another question, and I have to ask you to add a changelog entry to Active Record with the new json type for PostgreSQL. Just ping us after that, and we'll merge. Thanks! |
|
Thanks for suggestions! |
|
@carlosantoniodasilva and @rafaelfranca : I fixed the code following your suggestions. Thank you for you time on this! |
|
Great! Should you squash the commits? |
|
@rafaelfranca I squashed the commits and changed the commit message too. Tests still passing. Thanks! |
|
This is somehow out of date. CHANGELOGs! /me shakes his fist. |
|
Yes. Please rebase it. |
This implements the support to encode/decode JSON data to/from database and creating columns of type JSON using a native type [1] supported by PostgreSQL from version 9.2. [1] http://www.postgresql.org/docs/9.2/static/datatype-json.html
|
@rafaelfranca Sorry, I hope this is OK now. Thanks @steveklabnik to point me that. |
AR supporting new JSON data type on PostgreSQL >= 9.2
|
Thanks for accepting this! |
Hello all!
The next PostgreSQL version (9.2) will supports a native JSON type. Once the 9.2 version will be release soon I think that would be nice if AR supports it too.
Before started this, I searched for someone that could be working on this already, and tweeted @tenderlove asking if he remembers about someone doing this job, since seems that nobody is working on this I'm sending this pull request and I'd like to know your opinions about this feature and about my implementation. I marked two "FIXMEs" that could be a DRY candidate, IMO. Maybe should AR:Store be changed too?
Thanks.