node-mysql2: Incorrect arguments to mysqld_stmt_execute (MySQL 8.0.22)

mysql => 8.0.22 mysql2 => 2.2.5

const statement = SELECT moment.id as id, moment.content as content, moment.createAt createTime, moment.updateAt updataTime, JSON_OBJECT('id', users.id, 'username', users.username) author FROM moment LEFT JOIN users ON moment.user_id = users.id LIMIT ? OFFSET ?;

connection.execute(statement, [size + “”, page + “”])

replace number with characters

@sidorares, just FYI, instead of passing javascript number type, if value is passed as “string” (even though column in int), it works!

I have the same problem since i updated mysql from 8.0.21 -> 8.0.22. I am pretty sure that my queries are alright cause i didnt have any issues before version upgrade. I`ve got ubintu (20.04) which I updated yesterday and after installing the update this error started. I tested the queries on older version (before yesterdays update) and it seems to work without any problem. Any advice?

A workaround that I found yesterday was, pasring the bindedparms to string as such: bindedparams.map(i => i.toString()) ; If the params are left as integers the error appears

{ code: ‘ER_WRONG_ARGUMENTS’, errno: 1210, sqlState: ‘HY000’, sqlMessage: ‘Incorrect arguments to mysqld_stmt_execute’ }

@mikiU2022 its still November on my calendar, but thanks for the ping

As @ruiquelhas has stated all the numbers are passed by the driver as DOUBLE types. I know, that JavaScript sucks at dealing with integers, but there is a range of integers that can be safely represented using JavaScript’s number.

My proposal is to pass those safe integers as integers (therefore LONGLONG) and not as doubles. So the current code (lib/packets/execute.js) that now is:

case 'number':
  type = Types.DOUBLE;
  length = 8;
  writer = Packet.prototype.writeDouble;
  break;

would become something like:

case 'number':
  if (Number.isSafeInteger(value)) {
    type = Types.LONGLONG;
    length = 8;
    writer = Packet.prototype.writeInt64; // not implemented yet
  } else {
    type = Types.DOUBLE;
    length = 8;
    writer = Packet.prototype.writeDouble;
  }
  break;

This would solve this issue (I’ve tested locally) and maybe prevent many other ones. Am I missing something? What do you think?

I just noticed that this error only occurs for me in the LIMIT and OFFSET clauses. I thought that these would expect integers, but they apparently do not.

At first I thought all integers would have to be cast to strings, but this turns out to NOT be the case. Again only the LIMIT and OFFSET clauses. Hope this helps someone else realize that fixing this across a code base is not a huge task.

I’m still in the process of convincing management to release the code. Trust me, if it were solely my decision I’d have responded with a link to it. I’ll update here if something changes.

EDIT: Process is looking good, we might consider a release in the near future. Mind you the code is actually quite simple, so I don’t think it’s anything ground breaking. We’re mostly just casting numbers => string in the parameters when detected. The library does provide improved pooling logic we had to write for AWS RDS instances (auto-stale connections), so maybe something else is beneficial there.

EDIT 2: My company agreed for me to open source the repo. I’ll have it up in the coming days.

So we can’t use node-mysql2 + prepared statements on mysql 8.0.22, is this correct? :stumble:

@YangJ0605 did you actually apply some kind of monkey patch or did you rewrite all of your queries?

@sidorares my current workaround:

        const originalQuery = connection.execute;
        connection.execute = async function(...args) {
            // array handling
            const { query, replacements } = array2list(...args);
            args[0] = query;
            args[1] = replacements;

            // workaround for 8.0.22 => https://github.com/sidorares/node-mysql2/issues/1239
            for (const key in args[1]) {
                const value = args[1][key];
                if (typeof value === 'number')
                    args[1][key] = String(value);
            }

            // optional query logging
            if (logFunc)
                logFunc(query2executable(query, replacements));

            const result = await originalQuery.apply(connection, args);
            return result;
        }

Important Change: A prepared statement is now prepared only once, when executing PREPARE, rather than once each time it is executed.

@ruiquelhas can you clarify this?

That is also current behaviour or mysql2 driver: it sends COM_PREPARE only once for the first .execute() call and later if query is the same statement id is re used. I read server changelog the same way: when you do PREPARE command, after parsing it server uses body of the prepare argument and caches result ( previously each new PREPARE would generate new statement id ) - is that correct?

Yeah, the client flow is/was fine. That just mentions the change that happened in the server, which internally was always preparing statements once for each execution. That isn’t the case anymore, and the statements are now prepared effectively once (only when PREPARE is called i.e. when COM_STMT_PREPARE is sent). This caused some changes in the way dynamic parameters used in prepared statements are resolved (in particular when it comes to derive their type). Up until now, you could get away with sending different parameter types in the COM_STMT_EXECUTE, and the statement would basically be re-prepared using that type. This isn’t the case now, and even though I haven’t explored it in full, I guess, at least, COM_STMT_EXECUTE will have to use the types hinted by the COM_STMT_PREPARE Response.

If you are interested, the full write-up of those server changes is available here.

@BlockifyTechnologies short explanation: currently argument type is inferred from JS type ( you can see mapping here ). Previously this worked fine as the server was able to convert to type actually expected by the statement, but after version 8.0.22 this behaviour changed, see https://github.com/sidorares/node-mysql2/issues/1239#issuecomment-718827355

Workaround for you right now: force it to be sent as string, const values = ['0', '50'];. In the future we’ll add ability to explicitly set parameter type, the api would be something like this: const values = [mysql.types.LONGLONG(0), mysql.types.LONGLONG(50)]; - that way 0 and 50 are sent as LONGLONG instead of default DOUBLE. Also we plan to potentially change default type to be the one returned from prepare call, but this can be unreliable ( execute('SELECT ? as data') would have a type, likely LONG, set in a response but in reality its “unknown type” )

@pintarj I’m planning to start working on this soon, the intention is to use type from prepare response type hints - in your example prepare likely returned integer type for that parameter

I have discovered that my interim fix has issues, at least when tested with Percona Server 8.0.23, Percona XtraDB Cluster 8.0.23 and presumably that same official MySQL version.

Having looked at the general query log, it’s clear that the server is frequently unnecessarily re-preparing statements. This causes a performance hit and a constant increase in prepared statement handles, as shown by status variable Prepared_stmt_count.

It seems that the proper solution needs to be implemented, in which node-mysql2 respects the the type hint provided in the COM_STMT_PREPARE response and converts data type of the parameters accordingly.

This issue is present with MariaDB 10.5.10 as well. I have been unable to find which MariaDB release that introduced the corresponding change to prepared statements.

I tried @vlasky’s interim fix but it’s insufficient in my case. Like @AnnAngela’s example we’re passing JSON values.

Update: I extended @vlasky’s patch with the equivalence of using JSON.stringify on object params. Currently evaluating the following monkeypatch applied at runtime to our docker image:

sed -i.org 's|number|number-MONKEYPATCH-https://github.com/vlasky/node-mysql2/commit/0357a637649d03b6c051f11fc317aca87d4cf4f9|' /usr/src/app/node_modules/mysql2/lib/packets/execute.js
sed -i 's|type = Types.JSON|// MONKEYPATCH-https://github.com/sidorares/node-mysql2/issues/1239#issuecomment-840986787 type = Types.JSON|' /usr/src/app/node_modules/mysql2/lib/packets/execute.js

I just switched my cloud MySQL database server provider, and faced this same problem.

My code runs on mysql2@2.2.5 + mysql-client 8.0.25-0ubuntu0.20.04.1, and the new provider only has mysql server 8.0.18 - Source distribution, and I got the problem.

(But I DID NOT face this problem with mysql server 8.0.22 - Source distribution which providing from the previous provider, so it seems like something changed from 8.0.18 to 8.0.22)

The problem occured while running connection.execute with json:

await connection.execute("INSERT INTO `__sth__` (`id`, `time`, `__json__`) VALUES (NULL, ?, ?);", [new Date().toISOString(), ["__some_json__"]]);

and my workaround is using JSON.stringify:

await connection.execute("INSERT INTO `__sth__` (`id`, `time`, `__json__`) VALUES (NULL, ?, ?);", [new Date().toISOString(), JSON.stringify(["__some_json__"])]);

But this workaround is annoying and I hope there wiil be a solution without any workaround.

mysql => 8.0.22 mysql2 => 2.2.5

const statement = SELECT moment.id as id, moment.content as content, moment.createAt createTime, moment.updateAt updataTime, JSON_OBJECT('id', users.id, 'username', users.username) author FROM moment LEFT JOIN users ON moment.user_id = users.id LIMIT ? OFFSET ?;

connection.execute(statement, [size + “”, page + “”])

replace number with characters

await conn.execute(query, [skip.toString(), pageSize.toString()]); work for me, thanks

Any update to this? Facing same error on 8.0.22 and on 8.0.19. Works with query, fails with execute (works properly on 8.0.14)

Just stumbled upon the same problem. A statement works fine when using query, but fails with execute:

Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '? ORDER BY video_id DESC LIMIT ?' at line 1

@alisson-acioli yes, to a degree. You can work around it by casting your data to a more compatible type ( for example, pass number as a string ). We plan to add api to set mysql type of a parameter explicitly

@dalalmj no update on my side, reading @ruiquelhas comment I guess we need to make sure that types used for serialising parameters are exactly those returned by COM_STMT_PREPARE response ( right now we use dynamically whatever is parameters - number / string / buffer etc ) - might be wrong, need to double check