allow setting the collation in auth handshake

[dvilaverde]

This PR is to allow the collation to be set during the auth handshake in order to avoid requiring calls to SET NAMES post connection.

[dveeden]

Looks ok, but might need a few more tests. I'll have a full review next week

[dvilaverde]

Looks ok, but might need a few more tests. I'll have a full review next week

Please let me know of an additional changes or tests you might need.

[dveeden]

I think this would overflow for some collations.

• 255 / 0xff / utf8mb4_0900_ai_ci would be fine
• 309 / 0x0135 / utf8mb4_0900_bin would not be

[dveeden]

Weirdly enough the protocol says this is 1 byte and that only the low 8-bits are put in this field. Not sure how that's going to work.

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

[dveeden]

A MySQL 8.0 Client only allows the user to set the charset, not the collation:

  --default-character-set=name 
                      Set the default character set.

And as all default collations are in the 0-255 range this works with the protocol.

mysql> SELECT MAX(ID) FROM information_schema.COLLATIONS WHERE IS_DEFAULT='Yes';
+---------+
| MAX(ID) |
+---------+
|     255 |
+---------+
1 row in set (0.00 sec)

[dveeden]

So this needs testing for collations that have an id that's >255.

Also what is the reason for doing this? Is it connection setup performance? Are you always using an UTF-8 collation or something else?

[dveeden]

mysql> SELECT *,ID&0x00ff FROM information_schema.COLLATIONS WHERE COLLATION_NAME IN('utf8mb4_ja_0900_as_cs','latin1_bin') ORDER BY ID;
+-----------------------+--------------------+-----+------------+-------------+---------+---------------+-----------+
| COLLATION_NAME        | CHARACTER_SET_NAME | ID  | IS_DEFAULT | IS_COMPILED | SORTLEN | PAD_ATTRIBUTE | ID&0x00ff |
+-----------------------+--------------------+-----+------------+-------------+---------+---------------+-----------+
| latin1_bin            | latin1             |  47 |            | Yes         |       1 | PAD SPACE     |        47 |
| utf8mb4_ja_0900_as_cs | utf8mb4            | 303 |            | Yes         |       0 | NO PAD        |        47 |
+-----------------------+--------------------+-----+------------+-------------+---------+---------------+-----------+
2 rows in set (0.00 sec)

So if only 1 byte gets send, both these collations will have the same value.

In [24]: c = mysql.connector.connect(host='127.0.0.1',port=3306,user="test",password="test",collation="utf8mb4_unicode_520_ci", ssl_disabled=True)

In [25]: cur = c.cursor()

In [26]: cur.execute("SHOW SESSION VARIABLES LIKE '%collation%'");

In [27]: cur.fetchall()
Out[27]: 
[('collation_connection', 'utf8mb4_unicode_520_ci'),
 ('collation_database', 'utf8mb4_0900_ai_ci'),
 ('collation_server', 'utf8mb4_0900_ai_ci'),
 ('default_collation_for_utf8mb4', 'utf8mb4_0900_ai_ci')]

In [28]: cur.close()
Out[28]: True

In [29]: c.close()

In [30]: c = mysql.connector.connect(host='127.0.0.1',port=3306,user="test",password="test",collation="utf8mb4_ja_0900_as_cs", ssl_disabled=True)

In [31]: cur = c.cursor()

In [32]: cur.execute("SHOW SESSION VARIABLES LIKE '%collation%'");

In [33]: cur.fetchall()
Out[33]: 
[('collation_connection', 'utf8mb4_ja_0900_as_cs'),
 ('collation_database', 'utf8mb4_0900_ai_ci'),
 ('collation_server', 'utf8mb4_0900_ai_ci'),
 ('default_collation_for_utf8mb4', 'utf8mb4_0900_ai_ci')]

In [34]: cur.close()
Out[34]: True

In [35]: c.close()

In [40]: c = mysql.connector.connect(host='127.0.0.1',port=3306,user="test",password="test",collation="latin1_bin", ssl_disabled=True)

In [41]: cur = c.cursor()

In [42]: cur.execute("SHOW SESSION VARIABLES LIKE '%collation%'");

In [43]: cur.fetchall()
Out[43]: 
[('collation_connection', 'latin1_bin'),
 ('collation_database', 'utf8mb4_0900_ai_ci'),
 ('collation_server', 'utf8mb4_0900_ai_ci'),
 ('default_collation_for_utf8mb4', 'utf8mb4_0900_ai_ci')]

So, the protocol docs are wrong and Wireshark is also not decoding this properly. I'll follow up on both.

This can actually be two bytes (see the "Unused", which isn't all zeros in one of the cases)

[dveeden]

mysql/mysql-server#541

[dveeden]

https://gitlab.com/wireshark/wireshark/-/merge_requests/15464

[dvilaverde]

So this needs testing for collations that have an id that's >255.

Also what is the reason for doing this? Is it connection setup performance? Are you always using an UTF-8 collation or something else?

Hi yes our reason to do this is we have an application, using latin1_general_ci and MariaDB 5.5.x, that is sensitive to latency and we setup and tear down connections a a lot, primarily because there is a large number of MariaDB instances that application needs to interact with. Ideally we'd like reduce the connection setup time as much as possible.

[dveeden]

Maybe check with Wireshark if there are any more roundtrips you can avoid

[dvilaverde]

I believe I've addressed the > 255 collation ID and added a test to assert the bytes are correctly set in the auth handshake. I am hoping I've understood your comments and feedback correctly on this PR and addressed all the issues. Please let me know if I've misunderstood and you were looking for something else.

[dvilaverde]

Let me know if you prefer to follow the documentation for the protocol and limit the use of collation in the auth handshake to collations with an ID < 255. This would allow users to set the mysql 8.x utf8mb4_0900_ai_ci default if needed while remaining true to the protocol.

[dveeden]

It might be possible always encode the collation.ID to a 2 byte integer, if the right endianness is used. That might simplify the code a bit

[dveeden]

@lance6716 PTAL

[lance6716]

I'll take a look tomorrow

[lance6716]

rest lgtm

[dvilaverde]

rest lgtm

I've addressed the feedback from the review.