-
-
Notifications
You must be signed in to change notification settings - Fork 610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Incorrect arguments to mysqld_stmt_execute (MySQL 8.0.22) #1239
Comments
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
} |
Could you try to prepare from mysql cli?
trying to make sure sql syntax is valid ( there are limitation on to what can be a placeholder in PS ) |
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? |
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 { |
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:
Variables above being |
@ruiquelhas could you comment on this 8.0.21 -> 8.0.22 incompatibility? |
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. |
@perry-mitchell what's the column datatype of |
@ruiquelhas Confirming that |
@perry-mitchell I mean on the table column itself. The MySQL column datatype. Which one of these? |
@ruiquelhas |
Just stumbled upon the same problem. A statement works fine when using
|
as a matter of fact, something's weird is on mysql side, working query cannot be prepared:
|
@bkarlson try this:
|
well okay, that was a lame act on my side with mysql, yet this statement works with const sqlVideos = `SELECT video_id FROM videos ORDER BY video_id DESC LIMIT ?`;
pool.query(sqlVideosToProcess, [batchSize], async (err, rows) => {.. |
@bkarlson probably not relevant to the topic, but why async callback? |
what error with execute? |
I'm just refactoring old |
yes |
@perry-mitchell what about the datatype of the |
@sidorares from a preliminary analysis, I'm not able to reproduce this issue with the
From what I can tell, one of the problems is that when encoding the In this specific case, if we force the driver to encode the value using a 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 It's a |
@ruiquelhas can you clarify this? That is also current behaviour or mysql2 driver: it sends COM_PREPARE only once for the first |
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 |
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 If you are interested, the full write-up of those server changes is available here. |
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 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 |
@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. |
- [x] Changes code to fix: sidorares/node-mysql2#1239
Hello, is this problem still persisting? I am passing this now on 08/2023 |
@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 |
this problems persist... it's dec/2023..... |
@mikiU2022 its still November on my calendar, but thanks for the ping |
const sql = `SELECT * FROM DB.Users WHERE uid > ? ORDER BY uid LIMIT ?`;
const values = [0, 50];
try {
const [results] = await connection.execute(sql, values);
console.log("SQL Result: ", results);
} catch (error) {
console.error("Error executing SQL query:", error);
} Why would a query as simple as this give me the error of incorrect arguments ? |
@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 #1239 (comment) Workaround for you right now: force it to be sent as string, |
@sidorares |
We wrote a query wrapper in house to do this automatically.. and haven't had to think about it since. This particular problem can be abstracted away somewhat easily :) |
@perry-mitchell please share it! |
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. |
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 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. |
hey @fires3as0n thanks for a friendly nudge 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 |
@perry-mitchell where can I find your repo with the node-mysql2 query wrapper code? |
pageSize = 8; write query like this. |
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:
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 theexecute
method on themysql2/promise
library isSELECT * 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 ofexecute
, 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.
The text was updated successfully, but these errors were encountered: