Skip to content

Conversation

@timkrins
Copy link

@timkrins timkrins commented Jun 2, 2023

We have been using mobility for a few years using the JSON backend with a MySQL 8 JSON column.

I see a few people (maybe just kule) over the years have put together PRs, but #271 is still marked WIP.

The thing I am most unsure about in this PR is the visit_Mobility_Plugins_Arel_Nodes_JsonDashDoubleArrow function.
Maybe moving that quoting somewhere else would be better - I notice PR #271 from a few years ago has it elsewhere.

As ActiveRecord 4.2 does not support MySQL json columns, I have excluded that test - but I guess it would be possible to run the JSON test only against ActiveRecord 4.2+ and leave support for non-JSON columns... but Rails 4.2.5.1 is 7 years old now...

@timkrins timkrins changed the title MySQL JSON backend MySQL JSON backend for ActiveRecord Jun 3, 2023
@shioyama
Copy link
Owner

shioyama commented Jun 3, 2023

Thanks very much for sharing this! I noticed you pulled the commit fixing the Sequel failures, I need to fix that (it "fixes" the failures but doesn't really doing it correctly.)

Once I do that I'll have a look at this.

@shioyama
Copy link
Owner

shioyama commented Jun 3, 2023

but Rails 4.2.5.1 is 7 years old now...

Oh yeah I thought I'd removed that one from the builds. I'll remove it so don't worry about that 👍 Probably should also remove Rails 5 too actually.

@timkrins
Copy link
Author

timkrins commented Jun 3, 2023

I did pull that branch, as I noticed the tests were green, and wanted to run the full CI matrix on this PR.

I actually had a quick look at adding Sequel support for the JSON column too, and not just adding the ActiveRecord support, but I've never used Sequel and it doesn't seem to support JSON columns without another gem.

@timkrins
Copy link
Author

timkrins commented Jul 10, 2023

I just noticed some discussion on another issue about the default column value {} being required in JSON stores...
MySQL doesn't have defaults for JSON columns.
What should I be doing here? I haven't noticed any particular problem with not having a default but I am aware there might be something that I'm missing.
Possibly related to
https://github.com/shioyama/mobility/wiki/Postgres-Backends-%28Column-Attribute%29#railsactiverecord
(but it doesn't explain the reasons why it is required)
and an issue mentioning this default
#544

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

Successfully merging this pull request may close these issues.

2 participants