Add a native JSON data type support in MySQL #21110

Merged
merged 1 commit into from Aug 18, 2015

Conversation

Projects
None yet
8 participants
@kamipo
Member

kamipo commented Aug 3, 2015

As of MySQL 5.7.8, MySQL supports a native JSON data type.

http://dev.mysql.com/doc/refman/5.7/en/json.html

Example:

create_table :json_data_type do |t|
  t.json :settings
end
@jeremy

This comment has been minimized.

Show comment
Hide comment
@jeremy

jeremy Aug 15, 2015

Member

👍 nice @kamipo !

Member

jeremy commented Aug 15, 2015

👍 nice @kamipo !

@sahin

This comment has been minimized.

Show comment
Hide comment

sahin commented Aug 18, 2015

+1

@@ -2,32 +2,7 @@ module ActiveRecord
module ConnectionAdapters

This comment has been minimized.

@rafaelfranca

rafaelfranca Aug 18, 2015

Member

We can remove this whole file and use Type::Json where we used OID::Json

@rafaelfranca

rafaelfranca Aug 18, 2015

Member

We can remove this whole file and use Type::Json where we used OID::Json

This comment has been minimized.

@kamipo

kamipo Aug 18, 2015

Member

OID::Json is a base class of OID::Jsonb.
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/postgresql/oid/jsonb.rb#L5
Do I need to change to class Jsonb < Type::Json?

@kamipo

kamipo Aug 18, 2015

Member

OID::Json is a base class of OID::Jsonb.
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/postgresql/oid/jsonb.rb#L5
Do I need to change to class Jsonb < Type::Json?

This comment has been minimized.

@rafaelfranca

rafaelfranca Aug 18, 2015

Member

Yeah, looks good.

@rafaelfranca

rafaelfranca Aug 18, 2015

Member

Yeah, looks good.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 18, 2015

Member

Awesome! Just remove the OID class and we can merge it.

Member

rafaelfranca commented Aug 18, 2015

Awesome! Just remove the OID class and we can merge it.

@kamipo

This comment has been minimized.

Show comment
Hide comment
@kamipo

kamipo Aug 18, 2015

Member

Removed OID::Json!

Member

kamipo commented Aug 18, 2015

Removed OID::Json!

Add a native JSON data type support in MySQL
As of MySQL 5.7.8, MySQL supports a native JSON data type.

Example:

    create_table :json_data_type do |t|
      t.json :settings
    end

rafaelfranca added a commit that referenced this pull request Aug 18, 2015

Merge pull request #21110 from kamipo/mysql_json_support
Add a native JSON data type support in MySQL

@rafaelfranca rafaelfranca merged commit a316860 into rails:master Aug 18, 2015

@kamipo kamipo deleted the kamipo:mysql_json_support branch Aug 18, 2015

sgrif added a commit that referenced this pull request Aug 21, 2015

JSON is still an adapter specific type.
Several changes were made in #21110 which I am strongly opposed to.
(this is what I get for going on vacation. :trollface:) No type should
be introduced into the generic `ActiveRecord::Type` namespace, and
*certainly* should not be registered into the registry unconstrained
unless it is supported by *all* adapters (which basically means that it
was specified in the ANSI SQL standard).

I do not think `# :nodoc:` ing the type is sufficient, as it still makes
the code of Rails itself very unclear as to what the role of that class
is. While I would argue that this shouldn't even be a super class, and
that MySql and PG's JSON types are only superficially duplicated (they
might look the same but will change for different reasons in the
future).

However, I don't feel strongly enough about it as a point of contention
(and the biggest cost of harming the blameability has already occured),
so I simply moved the superclass into a namespace where its role is
absolutely clear.

After this change, `attribute :foo, :json` will once again work with
MySQL and PG, but not with Sqlite3 or any third party adapters.

Unresolved questions
--------------------

The types that and adapter publishes (at least those are unique to that
adapter, and not adding additional behavior like `MysqlString` should
probably be part of the adapter's public API. Should we standardize the
namespace for these, and document them?

@dfranciosi dfranciosi referenced this pull request in solidusio/solidus_braintree Dec 7, 2015

Closed

Check MySQL version in spree_credit_cards data json field #10

@NeMO84

This comment has been minimized.

Show comment
Hide comment
@NeMO84

NeMO84 Dec 22, 2015

Thanks for adding this support!! Is this feature only available in the 5.x versions?

NeMO84 commented Dec 22, 2015

Thanks for adding this support!! Is this feature only available in the 5.x versions?

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Dec 22, 2015

Member

Yes

Member

sgrif commented Dec 22, 2015

Yes

@akshah123

This comment has been minimized.

Show comment
Hide comment
@akshah123

akshah123 Jun 8, 2016

@sgrif are there any plans to add JSON support in 4.x branch? If not, would there be any interest in a PR that does that?

akshah123 commented Jun 8, 2016

@sgrif are there any plans to add JSON support in 4.x branch? If not, would there be any interest in a PR that does that?

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Jun 8, 2016

Member

No. Like stated in our maintenance policy, new features are no applied to
stable releases.
On qua, 8 de jun de 2016 at 11:28 Ankit Shah notifications@github.com
wrote:

@sgrif https://github.com/sgrif are there any plans to add JSON support
in 4.x branch?


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#21110 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAC66IAsvgFFFQVVyA9GUq0TOAc9L_WBks5qJt-KgaJpZM4Fk0OL
.

Member

rafaelfranca commented Jun 8, 2016

No. Like stated in our maintenance policy, new features are no applied to
stable releases.
On qua, 8 de jun de 2016 at 11:28 Ankit Shah notifications@github.com
wrote:

@sgrif https://github.com/sgrif are there any plans to add JSON support
in 4.x branch?


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#21110 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAC66IAsvgFFFQVVyA9GUq0TOAc9L_WBks5qJt-KgaJpZM4Fk0OL
.

@metaskills metaskills referenced this pull request in rails-sqlserver/activerecord-sqlserver-adapter Jun 12, 2016

Closed

Ensure `supports_json?` Works #485

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