node-mysql2 icon indicating copy to clipboard operation
node-mysql2 copied to clipboard

Incorrect arguments to mysqld_stmt_execute (MySQL 8.0.22)

Open perry-mitchell opened this issue 5 years ago • 60 comments

Hi, love this library!

Have been using it successfully across a large number of projects and it's worked flawlessly for me so far. Just now I've seen that I have a single test case failing on the CI server (Gitlab), MySQL 8, failing with the following error:

VError: Failed executing query: \"SELECT * FROM message WHERE chat_id = ? ORDER BY sent DESC LIMIT ? OFFSET ?\" [01ensmg6m4dea0gh7gsjgaa5gb, 50, 0]: Incorrect arguments to mysqld_stmt_execute
    at ConnectionPool.<anonymous> (/data/node_modules/@my-company/mysql-connection-pool/dist/ConnectionPool.js:128:27)
    at Generator.throw (<anonymous>)
    at rejected (/data/node_modules/@my-company/mysql-connection-pool/dist/ConnectionPool.js:6:65)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
caused by: Error: Incorrect arguments to mysqld_stmt_execute
    at PromiseConnection.execute (/data/node_modules/mysql2/promise.js:110:22)
    at ConnectionPool.<anonymous> (/data/node_modules/@my-company/mysql-connection-pool/dist/ConnectionPool.js:117:58)
    at Generator.next (<anonymous>)
    at fulfilled (/data/node_modules/@my-company/mysql-connection-pool/dist/ConnectionPool.js:5:58)

Now this error is wrapped by my own logic, but the cause I think it quite clear: Incorrect arguments to mysqld_stmt_execute. The query being passed to the execute method on the mysql2/promise library is SELECT * FROM message WHERE chat_id = ? ORDER BY sent DESC LIMIT ? OFFSET ? and the values are ["01ensmg6m4dea0gh7gsjgaa5gb", 50, 0]. For some reason this only fails on the CI, and not locally on any of my (or my colleagues') machines.

If I change this to query instead of execute, it works on the CI. If I form this query manually using the values that were passed-in, and run it, it also works.

Any idea what's happening here? Is there some failure in how the parameters are being transferred to the MySQL service before being inserted?

EDIT 1: I'm also using v2.2.5 of the library

EDIT 2: Seems after the suggestions made here that the issue, at least for me, is only with mysql2 and MySQL server 8.0.22. 8.0.21 seems to work fine.

perry-mitchell avatar Oct 29 '20 07:10 perry-mitchell

Not sure it helps, but this is the usage:

export async function getChatMessages(
    chatID: string,
    limit: number,
    offset: number = 0,
    connection = getConnection()
): Promise<Array<MessageSlow>> {
    const [
        rows
    ] = await connection.execute(
        `SELECT * FROM message WHERE chat_id = ? ORDER BY sent DESC LIMIT ? OFFSET ?`,
        [chatID, limit, offset]
    );
    // Snip
}

perry-mitchell avatar Oct 29 '20 07:10 perry-mitchell

Could you try to prepare from mysql cli?

mysql> PREPARE stmt1 FROM 'SELECT * FROM message WHERE chat_id = ? ORDER BY sent DESC LIMIT ? OFFSET ?';

trying to make sure sql syntax is valid ( there are limitation on to what can be a placeholder in PS )

sidorares avatar Oct 29 '20 08:10 sidorares

hm, re reading your text, it works on local machine. Can you double check your local mysql server version is exactly the same as on CI?

sidorares avatar Oct 29 '20 08:10 sidorares

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' }

dimitar3web avatar Oct 29 '20 08:10 dimitar3web

Ok, that was a good recommendation @sidorares - I was not using the same version locally. I've updated my local to MySQL 8.0.22 and now it fails on many queries:

VError: Failed executing query: "
    SELECT
        m.chat_id,
        MAX(m.sent) AS sent,
        c.channel,
        c.is_completed
    FROM message AS m
    INNER JOIN chat AS c ON c.id = m.chat_id
    WHERE
        sent <= ? AND
        c.is_completed = 0
    GROUP BY m.chat_id
    LIMIT ?
" [2020-10-29 10:11:45, 10]: Incorrect arguments to mysqld_stmt_execute

Variables above being ["2020-10-29 10:11:45", 10]

perry-mitchell avatar Oct 29 '20 10:10 perry-mitchell

@ruiquelhas could you comment on this 8.0.21 -> 8.0.22 incompatibility?

sidorares avatar Oct 29 '20 10:10 sidorares

Yeah, there were some major changes with regards to how prepared statements work in the server (type inferences and whatnot), and unfortunately some specific queries might break. I'll look at this specific example as well. Thanks for the mention @sidorares.

ruiquelhas avatar Oct 29 '20 10:10 ruiquelhas

@perry-mitchell what's the column datatype of chat_id? Just so I can re-create the exact same scenario.

ruiquelhas avatar Oct 29 '20 11:10 ruiquelhas

@ruiquelhas chat_id is a string, 26 chars. The values being inserted in that first query are ["01ensmg6m4dea0gh7gsjgaa5gb", 50, 0].

Confirming that 8.0.21 works for me, and 8.0.22 presents these prepare statement errors.

perry-mitchell avatar Oct 29 '20 11:10 perry-mitchell

@ruiquelhas chat_id is a string, 26 chars. The values being inserted in that first query are ["01ensmg6m4dea0gh7gsjgaa5gb", 50, 0].

Confirming that 8.0.21 works for me, and 8.0.22 presents these prepare statement errors.

@perry-mitchell I mean on the table column itself. The MySQL column datatype. Which one of these?

ruiquelhas avatar Oct 29 '20 11:10 ruiquelhas

@ruiquelhas varchar(30) sorry..

perry-mitchell avatar Oct 29 '20 11:10 perry-mitchell

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

bkarlson avatar Oct 29 '20 12:10 bkarlson

as a matter of fact, something's weird is on mysql side, working query cannot be prepared:

mysql> PREPARE stmt SELECT video_id FROM videos ORDER BY video_id DESC LIMIT 10;
ERROR 1064 (42000): 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 'SELECT video_id FROM videos ORDER BY video_id DESC LIMIT 10' at line 1


mysql> SELECT video_id FROM videos ORDER BY video_id DESC LIMIT 10;
+-----------+
| video_id  |
+-----------+
[...]
+-----------+
10 rows in set (0.00 sec)

bkarlson avatar Oct 29 '20 13:10 bkarlson

@bkarlson try this:

mysql> PREPARE stmt1 FROM 'SELECT video_id FROM videos ORDER BY video_id DESC LIMIT 1';

sidorares avatar Oct 29 '20 13:10 sidorares

well okay, that was a lame act on my side with mysql, yet this statement works with query but not with execute

const sqlVideos = `SELECT video_id FROM videos ORDER BY video_id DESC LIMIT ?`; 
pool.query(sqlVideosToProcess, [batchSize], async (err, rows) => {..

bkarlson avatar Oct 29 '20 13:10 bkarlson

@bkarlson probably not relevant to the topic, but why async callback?

sidorares avatar Oct 29 '20 13:10 sidorares

what error with execute? You have an error in your SQL syntax ?

sidorares avatar Oct 29 '20 13:10 sidorares

@bkarlson probably not relevant to the topic, but why async callback?

I'm just refactoring old mysql code to mysql2 with promises, and stumbled upon this issue. Now wondering whether the rest of my queries would work...

bkarlson avatar Oct 29 '20 13:10 bkarlson

what error with execute? You have an error in your SQL syntax ?

yes

bkarlson avatar Oct 29 '20 13:10 bkarlson

@perry-mitchell what about the datatype of the sent column?

ruiquelhas avatar Oct 29 '20 13:10 ruiquelhas

@sidorares from a preliminary analysis, I'm not able to reproduce this issue with the mysql CLI or other wire protocol implementations with prepared statement support. Again, this seems to be related to the changes in how prepared statements work with regards to type inference and whatnot. As described in the 8.0.22 server release notes.

Important Change: A prepared statement is now prepared only once, when executing PREPARE, rather than once each time it is executed. In addition, a statement inside a stored procedure is also now prepared only once, when the stored procedure is first executed. This change enhances performance of such statements, since it avoids the added cost of repeated preparation and rollback of preparation structures, the latter being the source of several bugs.

From what I can tell, one of the problems is that when encoding the COM_STMT_EXECUTE this driver is mapping all JavaScript number values to MYSQL_TYPE_DOUBLE and ignoring the type hint available in the COM_STMT_PREPARE Response.

In this specific case, if we force the driver to encode the value using a MYSQL_TYPE_LONGLONG type (which is the one suggested by the server after the prepare stage), it seems to work fine.

In any case, I'm still trying to figure out the entire thing. In the meantime, we can take this "offline" and discuss how can this be addressed from the client POV, because the server will own these changes.

ruiquelhas avatar Oct 29 '20 15:10 ruiquelhas

what about the datatype of the sent column?

@ruiquelhas It's a timestamp

perry-mitchell avatar Oct 29 '20 17:10 perry-mitchell

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?

sidorares avatar Oct 29 '20 23:10 sidorares

mapping all JavaScript number values to MYSQL_TYPE_DOUBLE and ignoring the type hint available in the COM_STMT_PREPARE Response.

yes. Initially this driver was using string type for all parameters relying on server to do all the translation to actual expected types, and right now types are inferred from JS parameters ( and not on based on what we know server expects as a parameter ) - added in #353 and #705

also ref https://github.com/sidorares/node-mysql2/pull/516#issuecomment-459656963 ( I'm sure I discussed "we need to use types from prepare response to serialize parameters" somewhere before but can't find it )

sidorares avatar Oct 29 '20 23:10 sidorares

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.

ruiquelhas avatar Oct 30 '20 09:10 ruiquelhas

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)

dalalmj avatar Jan 14 '21 06:01 dalalmj

@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

sidorares avatar Jan 14 '21 07:01 sidorares

@dalalmj if it happens on 8.0.19 then it's probably not related to this specific issue. The changes I mentioned were only introduced by the MySQL server in 8.0.22.

ruiquelhas avatar Jan 14 '21 09:01 ruiquelhas

@ruiquelhas, my mistake, I tried installing 8.0.19, but now see that apt-get always installs latest 8.0.22.

dalalmj avatar Jan 14 '21 09:01 dalalmj

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

dalalmj avatar Jan 14 '21 09:01 dalalmj