Add mysql json support (wip)

[kule]

This is an initial stab at getting mysql json tests passing (related #226).

This isn't ready for use yet and is more to get discussion going - this will break postgres specs as I've commented out pg_opts for now.

Things that needed changing to get the mysql tests passing:

• Rather then 'en', json keys must be in the format '$.en'.
• Because dashes aren't valid ECMAScript identifiers these must be escaped so '$.en-GB' needs to become '$."en-GB"'
• You can't have a default value on a json column
• The first set of like/ilike specs (with case-insensitivity) fail for mysql because the json appears to be case-sensitive regardless of collation (EDIT: so currently I've skipped these for mysql). Would have to use lower() by the looks of things...

TODO - would appreciate some thoughts or ideas on these:

• Uncomment pg_opts and refactor so mysql_opts can be loaded too
• Refactor quoting - this'll probably need escaping properly
• Decide whether to ignore or fix the json case-insensitivity tests

[shioyama]

Thanks very much! Really happy to see this, looks great. I'll have a look in the next few days and get back about where to go from here.

[kule]

Cool I started looking at the changes required for travis - it’ll need changing to trusty with sudo required to get MySQL 5.7 (which has json support).

Looks like things don’t play so nice in rails 4 either so I’ll have a look see what’s needed.

[shioyama]

Looks like things don’t play so nice in rails 4 either so I’ll have a look see what’s needed.

Don't worry about that, it's absolutely fine not to support Rails 4 on this feature.

You can use the metadata rails_version_geq: 5.0 to mark specs for mysql json that fail in Rails 4, like this:

mobility/spec/support/shared_examples/serialization_examples.rb

Lines 57 to 65 in 1686f5d

	it "deletes keys with nil values when saving", rails_version_geq: '5.0' do
	backend.write(:en, "foo")
	expect(instance.read_attribute(column1)).to match_hash({ en: "foo" })
	backend.write(:en, nil)
	expect(instance.read_attribute(column1)).to match_hash({ en: nil })
	instance.save
	expect(backend.read(:en)).to eq(nil)
	expect(instance.read_attribute(column1)).to match_hash({})
	end

[shioyama]

Because dashes aren't valid ECMAScript identifiers these must be escaped so '$.en-GB' needs to become '$."en-GB"'

I believe underscore is allowed as an identifier, so couldn't we use '$.en_gb' instead? i.e. Mobility.normalize_locale(locale).

You can't have a default value on a json column

Hmm. With postgres you can set the deafault to an empty hash, is this not possible with MySQL? It makes things slightly simpler if you can do this.

The first set of like/ilike specs (with case-insensitivity) fail for mysql because the json appears to be case-sensitive regardless of collation (EDIT: so currently I've skipped these for mysql). Would have to use lower() by the looks of things...

That's totally fine!

[shioyama]

Uncomment pg_opts and refactor so mysql_opts can be loaded too

I think we should probably namespace the pg-specific constants, maybe we can just prepend Pg to their names?

i.e. instead of Mobility::Arel::Nodes::Json, we call that Mobility::Arel::Nodes::PgJson, etc? Then you could also define Mobility::Arel::Nodes::MysqlJson maybe? Not sure about the naming.

[shioyama]

It's great that you're trying to support a container format for MySQL Json, but I think it will make things a bit complicated for this first PR. How about removing this and leaving it for another PR later?

[shioyama]

@kule I made some quick comments, let me know what you think. I'll try to have another look later this week.

[kule]

@shioyama thanks I'll see if I can get time towards the end of the week to make these changes.

RE default values - no it's a limitation with MySQL unfortunately: https://dev.mysql.com/doc/refman/8.0/en/json.html (about 4th paragraph down). MySQL has the same issue with TEXT & BLOB fields, I think it's due to the way it stores them.

[shioyama]

Great thanks!

it’ll need changing to trusty with sudo required to get MySQL 5.7 (which has json support).

Also, for this yes please update .travis.yml to use sudo so the tests run. I'm not sure if we'll actually leave this when merging since it will slow down the whole suite, but anyway first let's make sure these changes pass specs. I don't know if there is a way with travis to run some specs with sudo and others without, I suspect not...