sidorares / node-mysql2

:zap: fast mysqljs/mysql compatible mysql driver for node.js
https://sidorares.github.io/node-mysql2/
MIT License
4.08k stars 622 forks source link

JSON doesn't auto parse on MariaDB #1287

Open bluewave41 opened 3 years ago

bluewave41 commented 3 years ago

Given a simple users table

field type
userId int(10)
saved longtext utf8mb4_bin

With a single row

userId: 1, saved: {"value": 2}

    const mysql = require('mysql2');

    async function start() {
        const connection = mysql.createConnection({
            host: 'localhost',
            user: 'root',
            database: 'database'
        });
        connection.query('SELECT saved FROM users WHERE userId = 1', function(err, results, fields) {
            console.log(results);
        })
    }

    start();

Will return

[ TextRow { saved: '{"value": 1}' } ] in MariaDB [ TextRow { saved: { value: 1 } } ] in MySQL

Gricardov commented 3 years ago

Same problem here chamo

sidorares commented 3 years ago

@bluewave41 @Gricardov what's column type is returned from mariadb server? Text of returned row field is parsed into json if field type is 0xf5 JSON - see https://github.com/sidorares/node-mysql2/blob/d74558b605162156b813248a024f7559785de6fb/lib/parsers/text_parser.js#L63 If mariadb uses different type code for JSON columns this could explain observed behaviour

Ark42 commented 1 year ago

A column defined as JSON DEFAULT NULL on MySQL (AWS) is:

    characterSet: 63,
    encoding: 'binary',
    columnType: 245,
    type: 245,

while the same column on MariaDB 10.6.12 (Ubuntu 22.04) is:

    characterSet: 45,
    encoding: 'utf8',
    columnType: 252,
    type: 252,

You can even see in MariaDB with SHOW CREATE TABLE that it just changed the table definition to:

colname longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL CHECK (json_valid(`colname`))

So it's literally being stored as LONGTEXT, but coming back over the wire as 0xfc (BLOB / TEXT) with just a check constraint added to ensure your string is valid JSON...

I don't see how it would be possible to have node-mysql2 automatically know that THIS BLOB/TEXT needs to be JSON.parse()'d while other BLOB/TEXT columns should not...

Ark42 commented 1 year ago

It would actually be much more useful to myself personally, if there was an option like dateStrings but jsonStrings which could be enabled to cause the node-mysql2 module to NOT automatically JSON.parse() JSON fields on MySQL servers. I was building an application and testing locally with MariaDB, and we're already doing our own JSON.parse() on all fields which we know to be JSON. This automatic parsing in mysql2 actually broke our code when we tested it on AWS, because now it's getting double-parsed, which of course throws the old "SyntaxError: Unexpected token o in JSON at position 1" error.

sidorares commented 1 year ago

I don't see how it would be possible to have node-mysql2 automatically know that THIS BLOB/TEXT needs to be JSON.parse()'d while other BLOB/TEXT columns should not...

Well, I think "type: 245" should be a good / only signal for that. With mariadb unfortunately one would need to add typeCast with JSON.parse

Ark42 commented 1 year ago

Yes, type:245 would have made sense to automatically parse JSON, but I think it would be more valuable for most people to have their code work the same regardless of if the DB is connected to MySQL or MariaDB, so the JSON.parse would be better off in the application itself. Unfortunately, having it there means the code breaks on MySQL, but if mysql2 didn't do the JSON.parse, the same code would work the same for both DBs. I would very much like to have jsonStrings:true so that I can turn off the JSON.parse inside of mysql2.

I don't think the typeCast function works for mysql2 either, because it only seems to operate on query() but not execute() or the other way around, I forget. I abandoned trying to get the typecast function working because of that, and am just using a local fork of mysql2 with the JSON.parse removed for JSON columns.

VergeDX commented 7 months ago

mariadb connector considered this situation, they have a default open "autoJsonMap" option. Documention: https://github.com/mariadb-corporation/mariadb-connector-nodejs/blob/master/documentation/connection-options.md

sidorares commented 7 months ago

@Ark42 I see your point, happy to add jsonStrings flag ( default to false to keep current behavior ). When set to true JSON columns are returned as strings