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

Change the way types for prepared statement parameters are calculated

Open sidorares opened this issue 3 years ago • 3 comments

Currently types are inferred based on what is in the input (Date -> mysql date, js number -> mysql double, everything else - string ). This used to work ok until mysql server version 8.0.22

Calling execute('SELECT * from foo limit ?' , 1) results in Incorrect arguments to mysqld_stmt_execute` error. The type of the parameter server expects ( LONGINT ) is incompatible to what is actually sent ( DOUBLE ).

The fix is to use parameters column definitions returned from the previous prepare() call and use types from there instead of inferring type from the provided input

sidorares avatar Oct 11 '21 04:10 sidorares

ref #1239

sidorares avatar Oct 11 '21 10:10 sidorares

@vlasky when you have time can you try this brach to see if it fixes Incorrect arguments to mysqld_stmt_execute error for you?

It's a potentially backwards incompatible change in some scenarios, I'm thinking to release it as a major version bump. Maybe alpha release initially

sidorares avatar Oct 11 '21 21:10 sidorares

Hello @sidorares For what it's worth, I had the exact same issue as mentioned in #1239 and tried this branch with great success.

kimf avatar Apr 27 '22 08:04 kimf

Any updates on this?

mlucic avatar Nov 23 '22 16:11 mlucic

@mlucic unfortunately no progress here (but its quite high in my priority)

The way I want to implement this currently is different from this PR though.

The plan is:

  1. Introduce a "typed parameter" helpers which would hold the value you pass together with type information. Not sure about the exact api, but it might look like this:
const mediumintparam = mysql.TypedParameter.MEDIUMINT(123);
const floatparam =  mysql.TypedParameter.FLOAT(123);

conn.execute('select ?+? as sum', [mediumintparam, floatparam]);

when used via type helper the type sent with COM_EXECUTE is always what you specify

  1. when untyped js value is passed we have 2 options: infer from current value ( string / date / number / null / non-null object ) or use response from COM_PREPARE. The problem "type hint from the server" is that its not always accurate ( for example, there are scenarios when the server has no way of telling parameter type - select ?+? as sum ). I'll need to think when this hint might be useful and when not ( any feedback is appreciated! ), ideally we need a matrix where there is js type on one axis, COM_EXECUTE type hint on another axis mapped to what the driver will actually use for type

step 1) can be done now and would help solving current edge cases with a bit of manually added typings ( as in mysql types metadata in users code, not typescript )

after that we can add step 2 to make default map work with no surprises most of the time. Mysql is quite capable of casting back to a correct value on a server side, in earlier versions of this driver all .execute() parameters were converted to a string ( sometimes its even more efficient, "1" takes 2 bytes over the wire where double precision number is 8 )

example table:

Mysql type hint \ JS type string number bigint Date
LONGLONG VAR_STRING VAR_STRING VAR_STRING DATETIME
VAR_STRING VAR_STRING DOUBLE VAR_STRING DATETIME

I'm starting to think simple "always send (parameter.toString()) as VAR_STRING" unless the type is explicitly specified by user" might be actually the best behaviour

sidorares avatar Nov 24 '22 00:11 sidorares