Packet.readLengthCodedNumberExt exception with mysql 8

[wsporto]

I am getting the exception Packet.readLengthCodedNumberExt when I try to execute the prepare statement with a mysql 8 database. This works fine when I run with mysql 5.

Trace:
at Packet.readLengthCodedNumberExt (C:\dev\mysql2-error\node_modules\mysql2\lib\packets\packet.js:231:13)
    at Packet.readLengthCodedNumber (C:\dev\mysql2-error\node_modules\mysql2\lib\packets\packet.js:193:17)
    at new ColumnDefinition (C:\dev\mysql2-error\node_modules\mysql2\lib\packets\column_definition.js:28:33)
    at Prepare.readParameter (C:\dev\mysql2-error\node_modules\mysql2\lib\commands\prepare.js:80:17)
    at Prepare.execute (C:\dev\mysql2-error\node_modules\mysql2\lib\commands\command.js:45:22)
    at Connection.handlePacket (C:\dev\mysql2-error\node_modules\mysql2\lib\connection.js:490:32)
    at PacketParser.onPacket (C:\dev\mysql2-error\node_modules\mysql2\lib\connection.js:95:12)
    at PacketParser.executeStart (C:\dev\mysql2-error\node_modules\mysql2\lib\packet_parser.js:75:16)
    at Socket.<anonymous> (C:\dev\mysql2-error\node_modules\mysql2\lib\connection.js:102:25)
    at Socket.emit (node:events:527:28)

Code to reproduce:

const mysql = require('mysql2/promise');

/* 
docker run --name mysql-dev -e MYSQL_ROOT_PASSWORD=password -e MYSQL_DATABASE=mydb 
-p 3306:3306 mysql:8.0.33 --default-authentication-plugin=mysql_native_password
*/
async function run() {
    const conn = await mysql.createConnection({
        host: 'localhost',
        user: 'root',
        password: 'password',
        database: 'mydb'
    });
    await conn.query(`
        CREATE TABLE IF NOT EXISTS mytable1 (
            id int NOT NULL AUTO_INCREMENT,
            value int,
            PRIMARY KEY (id)
        ) ENGINE=InnoDB
    `);
    await conn.prepare('select value from mytable1 order by ?');
}

run().then(() => {
    console.log("ok");
});

Using mysql2 v3.3.3

[sidorares]

Looks like there is some logic missing in the prepare response deserialiser. I'm expecting a column definition where the packet can be EOF

https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_com_stmt_prepare.html

if num_columns > 0 and CLIENT_OPTIONAL_RESULTSET_METADATA is not set or if medatdata_follows is RESULTSET_METADATA_FULL num_columns packets will follow. Then a EOF_Packet will be transmitted, provided that CLIENT_DEPRECATE_EOF is not set.

Thanks for the report @wsporto and repro sample, I'm working on a fix

The issue can be seen even on default existing db after launching the container, await conn.prepare('select * from mysqluser order by ?'); fails in the same way

[sidorares]

@ruiquelhas
Not sure if this is a server bug / feature:

• I'm preparing select Host from user where host = ? order by ? query ( I thought parameters are not allowed in "order by ?" but apparently they are allowed )
• prepare header is "2 parameters, 1 field, metadata_follows: 1"

Now, if client is not setting DEPRECATE_EOF flag the response is 1 column definition for first parameter, EOF packet, 1 column definition for the column. I'd expect 2 parameters, EOF, and one column.

If the client sets DEPRECATE_EOF flag the response if just 2 column definition packets ( 1 for the first parameter, one for the column.

No DEPRECATE_EOF:

Header 0001000000010002000000000117000002
First param 03646566000000013f000ce000fcff0300fd00001f000005000003
EOF fe000002002b000004
First column 03646566056d7973716c0475736572047573657204486f737404486f73740ce000fc030000fe034000000005000005

With DEPRECATE_EOF:

Header 0001000000010002000000000117000002
Column definition 1: 03646566000000013f000ce000fcff0300fd00001f00002b000003
Column definition 2: 03646566056d7973716c0475736572047573657204486f737404486f73740ce000fc030000fe0340000000

Server version 8.0.33

[ruiquelhas]

I'm out of "office" for a few days but I'll give it a look ASAP.

[sidorares]

Friendly ping @ruiquelhas :)

[ruiquelhas]

@sidorares up to 8.0.22, ORDER BY ? was supposed to work when using an ordinal to identify the column. This is not the case anymore, because it's also no longer supported by the SQL standard.

Additionally, the fact that it even worked was probably due to the fact that, previously, the statement was always re-prepared when executed, which as we discussed before regarding a different issue, has been addressed in that same release.

For a prepared statement of the form SELECT expr1, expr2, ... FROM table ORDER BY ?, passing an integer value N for the parameter no longer causes ordering of the results by the Nth expression in the select list; the results are no longer ordered, as is expected with ORDER BY constant.

Although there is no error, the value for the placeholder is allowed but is "ignored", which is probably why the response contains just one parameter in your case (the one for host = ?).

Ultimately, this is neither a bug nor a feature, but simply a breaking change in 8.0.22.

With regards to CLIENT_DEPRECATE_EOF, I'm not sure if there is a specific question. But it seems to work according to what is described in the documentation, or maybe I'm missing something.

Hope it helps.

[sidorares]

So what's likely throwing the driver off is that prepare header reports 1 parameter (preparing select * from user order by ?):

00010000003300010000000005000002

{
id: 1,
fieldCount: 51,
parameterCount: 1,
warningCount: 0
}

followed by eof packet fe000002002b000003 which we try to deserialise as a parameter ( because of parameterCount: 1 )
followed by 51 column definition packets

[sidorares]

Extra check "if the packet is eof when we expect a parameter treat it as end of parameters and start reading columns if fieldCount > 0" should fix the issue ( I might add the same to columns just in case )

[sidorares]

@wsporto the fix should be in v3.4.5

[wsporto]

Very nice! I confirm it works now with v3.4.5.

[sidorares]

@wsporto repeating previous comments: you should probably also fix it on your side as well as ORDER BY ? is not standard sql anymore and might behave not the way you expect

[sidorares]

@ruiquelhas

while original issue was unrelated to CLIENT_DEPRECATE_EOF flag, I think this shows that CLIENT_DEPRECATE_EOF cannot be used reliably because it need the correct number of parameters and columns to be reported by the prepare header.

Current behaviour ( CLIENT_DEPRECATE_EOF ):

• read the header
• read parameters until 1) number == header.parameterCount OR 2) current packet is EOF
• in case of (1) in the previous step, read the packet and expect it to be EOF, in case of (2) skip this step
• read fields until number == header.fieldCount or current packet is EOF

When CLIENT_DEPRECATE_EOF is on:

• read the header
• read exactly header.parameterCount parameters
• read exactly header.fieldCount columns

This won't work when header is {parameterCount: 2, fieldCount: 1} but the actual packets sent by the server are "header, parameter 1, field 1"

[wsporto]

@sidorares , I use it in my library (typesql). From the typesql side it still works because although it accepts the non standard sql ORDER BY ? as a valid SQL, the library will not execute it. It will parse the query and generate code having the ORDER BY parameters interpolated (string concatenation) instead of passing them as query parameters.

For example, the user can write the SQL SELECT name FROM product ORDER BY ?. Typesql will parse it and genereate code with the query SELECT name FROM product ORDER BY ${escapeOrderBy(params.orderBy)}.

[ruiquelhas]

@sidorares let me see if I got it right. The problem here is the server reporting the wrong number of parameters in cases like ORDER BY ?, at least since 8.0.22 (given that before, it would somehow use the parameter if it was the corresponding column ordinal). Is this correct? I'll give it a look and reach out to maintainers of the other official connectors to understand how they are handling this issue, if that's indeed the case.