Details
-
Bug
-
Status: Closed (View Workflow)
-
Blocker
-
Resolution: Fixed
-
10.7(EOL)
-
None
Description
After MDEV-26572 two tests fail with an ASAN build:
READ of size 4 at 0x6290000e6f60 thread T5
|
#0 0x558dcc06be73 in my_strcoll_ascii_4bytes_found /home/buildbot/buildbot/build/mariadb-10.7.0/strings/ctype-ascii.h:111
|
#1 0x558dcc06d45a in my_strnncoll_utf8mb3_general_ci /home/buildbot/buildbot/build/mariadb-10.7.0/strings/strcoll.ic:226
|
#2 0x558dcbf06683 in hashcmp /home/buildbot/buildbot/build/mariadb-10.7.0/mysys/hash.c:373
|
#3 0x558dcbf05ef6 in my_hash_first_from_hash_value /home/buildbot/buildbot/build/mariadb-10.7.0/mysys/hash.c:288
|
#4 0x558dcbf05d11 in my_hash_first /home/buildbot/buildbot/build/mariadb-10.7.0/mysys/hash.c:262
|
#5 0x558dcbf05a04 in my_hash_search /home/buildbot/buildbot/build/mariadb-10.7.0/mysys/hash.c:235
|
#6 0x558dcac84412 in get_variable(st_hash*, st_mysql_const_lex_string*, bool) /home/buildbot/buildbot/build/mariadb-10.7.0/sql/item_func.cc:4634
|
#7 0x558dcac84924 in Item_func_set_user_var::set_entry(THD*, bool) /home/buildbot/buildbot/build/mariadb-10.7.0/sql/item_func.cc:4687
|
This is a minimum reproducible script:
SET NAMES utf8; |
set @arg00=1; |
SELECT @:=1e0; |
The problem was not really introduced by MDEV-26572, it only revealed the fact that hashcmp() in mysys/hash.c asubes strnncoll():
NOTES:
|
If length is 0, comparison is done using the length of the
|
record being compared against.
|
This quick patch fixes the problem:
diff --git a/mysys/hash.c b/mysys/hash.c
|
index abc11b42500..c0c60a8117d 100644
|
--- a/mysys/hash.c
|
+++ b/mysys/hash.c
|
@@ -369,6 +369,22 @@ static int hashcmp(const HASH *hash, HASH_LINK *pos, const uchar *key, |
{
|
size_t rec_keylength; |
uchar *rec_key= (uchar*) my_hash_key(hash, pos->data, &rec_keylength, 1);
|
+ if (hash->charset != &my_charset_bin) |
+ {
|
+ /* |
+ The "length == rec_keylength" is needed for backward compatibility,
|
+ to avoid things like partition names and plugin names to be
|
+ accent insensitive. We should eventually:
|
+ - Add a new utf8 accent sensitive case insensitive collation
|
+ - Change SQL object names (plugins, partitions, etc) to use
|
+ this new collation.
|
+ - Remove the length test from here
|
+ */
|
+ return length != rec_keylength || |
+ my_strnncoll(hash->charset,
|
+ (uchar*) rec_key, rec_keylength,
|
+ (uchar*) key, length);
|
+ }
|
return ((length && length != rec_keylength) || |
my_strnncoll(hash->charset, (uchar*) rec_key, rec_keylength,
|
(uchar*) key, rec_keylength));
|
This patch assumes that the length "0" is passed only on binary data, while in case of text string data a correct length value is passed.
All MTR tests passed with the patch. So this assumption seems to be true.
But the patch slows down a very hot function by adding a new if testing hash->charset . It should not be pushed AS IS. It's only a demo showing where the real problem is.
The real patch should fix all my_hash_search() calls not to pass 0 as length.
Attachments
Issue Links
- is duplicated by
-
-
- Closed
-
- relates to
-
MDEV-26572 Improve simple multibyte collation performance on the ASCII range
-
- Closed
-