sidorares / node-mysql2

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

Incorrect arguments to mysqld_stmt_execute (MySQL 8.0.22) #1239

Open perry-mitchell opened 4 years ago

perry-mitchell commented 4 years ago

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.

sidorares commented 11 months ago

@perry-mitchell please share it!

perry-mitchell commented 11 months ago

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.

fires3as0n commented 3 months ago

I am sorry, but 4 years have passed since this issue was raised, there is MySQL 9 out there, I'm using the latest version of node-mysql2 library and still have to apply this solution by damianobarbati in every project.

If this is not going to be fixed any time in the future, am thinking about just cloning the repo and disabling this case statement As a temporary solution, to avoid patching the execute in server side code, but I wonder, won't it break anything?

The library itself is great, no push, I understand that this is a minor problem and appreciate your time for the open source. Thank you.

sidorares commented 3 months ago

hey @fires3as0n thanks for a friendly nudge If you have a capacity for a proper solution I can try to guide you.

Initial step I see is to allow to manually specify the type. For example, we'll provide a value+type containers, and when you pass a value using that container the data type and serialisation format will match to that of the parameter.

example:

// not sure about naming, say we have StatementParameter exported with DOUBLE/LONG etc fields on them

connection.execute(`SELECT video_id FROM videos ORDER BY video_id DESC LIMIT ?`, StatementParameter.LONG(1))

execute command would be able to see type hint and use LONG instead of incorrect DOUBLE

vlasky commented 2 months ago

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.

@perry-mitchell where can I find your repo with the node-mysql2 query wrapper code?

Sohamnimbalkar07 commented 2 months ago

Not the standard Practice but you can try this.

pageSize = 8; const offset = (pageNumber - 1) * pageSize; const query = select column_names from table_name where some_column = ? and another_column = ? limit ${pagesize} offset ${offset} ; const [result] = await connection.execute(query, [column1, column2]);

write query like this.

anshvert commented 4 weeks ago

pageSize = 8; const offset = (pageNumber - 1) * pageSize; const query = select column_names from table_name where some_column = ? and another_column = ? limit ${pagesize} offset ${offset}; const [result] = await connection.execute(query, [column1, column2]);

@Sohamnimbalkar07 This Suprisingly Worked ! . Thanks