Skip to content
New issue

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

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

Already on GitHub? # to your account

Why is parse on fetch limited to sqlite? #46

Closed
pandapaul opened this issue Jan 19, 2017 · 7 comments
Closed

Why is parse on fetch limited to sqlite? #46

pandapaul opened this issue Jan 19, 2017 · 7 comments

Comments

@pandapaul
Copy link

https://github.com/seegno/bookshelf-json-columns/blob/master/src/index.js#L52 limits parsing on fetch to only sqlite and sqlite3. Why is this? Allowing parse on fetch with mysql as the client seems to work great for me, but I might be misunderstanding how to enable parse on fetch in some other way.

Is there another way I should be enabling parse on fetch with mysql as the client?

Thanks very much,
Paul

@ricardogama
Copy link
Collaborator

Hi @pandapaul.

This plugin was designed to fix the JSON columns issue with PostgreSQL, and only later introduced tests for SQLite. If I remember correctly we believed that there was no need to use this plugin with MySQL as the client handles all these issues itself.

However, can you provide a snippet of your code and schema to figure out if we should introduce MySQL on the test suite?

@pandapaul
Copy link
Author

Oh interesting. The knex migration I use for creating this table looks approximately like this:

exports.up = knex => knex.schema.createTable('aTable', (table) => {
  table.uuid('id').primary().notNullable()
  table.json('someJson')
})

And here's what the bookshelf model looks like:

module.exports = bookshelf.Model.extend({
  tableName: 'aTable',
  uuid: true,
}, {
  jsonColumns: ['someJson'],
})

With mysql as the client, table.json() simply creates a TEXT column, so I'd love to parse that on fetches. Changing that parseOnFetch logic to include mysql works great to accomplish this, but maybe I'm missing something that would allow me to accomplish this without the plugin?

@pandapaul
Copy link
Author

Perhaps the real problem is that table.json() creates a TEXT column: knex/knex#1036

I'll attempt to adjust my schema and let you know how it goes.

@ricardogama
Copy link
Collaborator

I think it's the same problem here knex/knex#1800.

@pandapaul
Copy link
Author

pandapaul commented Jan 19, 2017

Agreed. Changing the schema to use table.specificType() to force the column to be typed JSON doesn't help. Results in the same errors as above, which lines up with what is reported in the knex issue you mentioned. I think this validates the use of this plugin for mysql clients on both save and fetch.

@ricardogama
Copy link
Collaborator

@pandapaul Released as 2.1.0.

@pandapaul
Copy link
Author

Splendid. Thanks @ricardogama and @abelsoares

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

No branches or pull requests

2 participants