fix(mariadb): do not automatically parse JSON fields by checking meta

[lesion]

This is related to an #14800

The type of field returned by mariadb does not depend on the version of mariadb, but on the version of mariadb WHEN the table was created!

https://mariadb.com/kb/en/result-set-packets/#field-types
For a JSON column, the column type field will be MYSQL_TYPE_STRING, but the extended type will indicate 'json'.

the connector use this field to decide to parse or not the response:
https://github.com/mariadb-corporation/mariadb-connector-nodejs/blob/59da962ce4ae6338bb14ed8854a70526b429911f/lib/cmd/decoder/text-decoder.js#L98

during a select the connector put fields details inside a "meta" key that could be used to check if returned value from the connector is already parsed or not:
https://github.com/mariadb-corporation/mariadb-connector-nodejs/blob/f25081715c9e1f24a10017e70787108d0d2ced01/documentation/promise-api.md#array-result-sets

a gist to test things:
https://gist.github.com/lesion/af0c7f1370ab6eb05cafdabd5b39ac33

[ephys]

Is there a way to add a regression test for this? I'm thinking create a text column using queryInterface.createTable but have the associated model use DataTypes.JSON?

query.js is likely to be completely rewritten in the future and a regression test would ensure we don't lose this logic

[lesion]

Is there a way to add a regression test for this? I'm thinking create a text column using queryInterface.createTable but have the associated model use DataTypes.JSON?
query.js is likely to be completely rewritten in the future and a regression test would ensure we don't lose this logic

I think we can test this by manually creating a table with a raw query using the two type of field mariadb uses for JSON and check if the results of a findAll are the same, could be something like:

  await sequelize.query("DROP TABLE IF EXISTS test");
  await sequelize.query(`CREATE TABLE test (id INTEGER AUTO_INCREMENT PRIMARY KEY,
    value1 longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL,
    value2 longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL CHECK(json_valid(value2)))`)
  await sequelize.query("INSERT INTO test (value1, value2) VALUES(?, ?)", { replacements: [JSON.stringify({ key: 'value' }), JSON.stringify({ key: 'value' })] } )
  const Test = sequelize.define('test', {
    value1: DataTypes.JSON,
    value2: DataTypes.JSON,
  }, {
    freezeTableName: true,
    timestamps: false
   })

  const ret = await Test.findAll({raw: true})

  console.error(typeof ret[0].value1, typeof ret[0].value2)
  console.error(ret[0].value1, ret[0].value2)

/** output without patch
string object
{"key":"value"} { key: 'value' }
**/

/** output with patch
object object
{ key: 'value' } { key: 'value' }
**/

[lesion]

Is there a way to add a regression test for this? I'm thinking create a text column using queryInterface.createTable but have the associated model use DataTypes.JSON?

query.js is likely to be completely rewritten in the future and a regression test would ensure we don't lose this logic

could we merge this? I wrote a unit test too https://github.com/sequelize/sequelize/pull/15704/files#diff-6a7c61f5188b4fa30990dd0dac3a0990087521e6a9d4431e10beecce7967573bR129-R153

Sorry to bother you, I'm worried because mariadb 10.8 is in debian stable so just and apt upgrade and this bug comes out

[WikiRik]

Can you resolve the conflicts with main? We'll take a look soon so we can merge

[WikiRik]

Minor suggestions to keep the test suite a little bit cleaner but the change and test itself looks good to me

[lesion]

nope, I need to take a look at the new test structure

[WikiRik]

Good points from the linter

[WikiRik]

@ephys I'll give you a few days to look at it and then I'll merge

[ephys]

I'm late but I'm fine with this change. I don't think it's the role of query.js to parse things but considering it's already here it's fine, and we'll redesign query.js in a future update