Hi, Alexander, On Dec 12, Alexander Barkov wrote:
Hi, Sergei,
Thanks for your review. Please see my comments inline.
The new path version is here:
https://github.com/MariaDB/server/commit/ae0320daef76c7c00f1c7dddd2920afed43...
On 12/11/23 19:48, Sergei Golubchik wrote:
+} +static inline size_t my_casedn_str_latin1(char *str) +{ + return my_casedn_str_8bit(&my_charset_latin1, str); +}
may be my_charset_ascii? with an assert that it's indeed ascii? and a comment that it's used for generated temp table names and such
"ascii" is now not a fully built-in character set. It's generated from conf_to_src. We should make it fully built-in like my_charset_latin1 eventually.
For now, as we agreed on slack, I added a comment before every my_caseXX_str_latin1() call explaining that the argument is in fact a pure ASCII string, so using latin1 is fine.
Oh. I thought we agreed that you add it before the function *declaration*, not before every call. But ok, I'll see the patch, may be it's fine and not too much at all.
diff --git a/sql/lex_ident.h b/sql/lex_ident.h index f39273b7da5..db96e40cab8 100644 --- a/sql/lex_ident.h +++ b/sql/lex_ident.h + if (dbls.length > SAFE_NAME_LEN) + { + sql_print_warning(ER_THD(thd, ER_WRONG_DB_NAME), dbls.str);
It's an error log, should every message be localized according to connection-specific locale?
Done. Fixed to ER_DEFAULT(ER_WRONG_DB_NAME).
And removed thd as a function argument, I hope
diff --git a/storage/perfschema/pfs_program.cc b/storage/perfschema/pfs_program.cc index de456610519..6c35a745d30 100644 --- a/storage/perfschema/pfs_program.cc +++ b/storage/perfschema/pfs_program.cc @@ -118,31 +118,20 @@ static void set_program_key(PFS_program_key *key, */
char *ptr= &key->m_hash_key[0]; + const char *end= ptr + sizeof(key->m_hash_key) - 1;
ptr[0]= object_type; ptr++;
if (object_name_length > 0) - { - char tmp_object_name[COL_OBJECT_NAME_SIZE + 1]; - memcpy(tmp_object_name, object_name, object_name_length); - tmp_object_name[object_name_length]= '\0'; - my_casedn_str(system_charset_info, tmp_object_name); - memcpy(ptr, tmp_object_name, object_name_length); - ptr+= object_name_length; - } + ptr+= system_charset_info->casedn(object_name, object_name_length, + ptr, end - ptr); ptr[0]= 0; ptr++;
if (schema_name_length > 0) - { - char tmp_schema_name[COL_OBJECT_SCHEMA_SIZE + 1]; - memcpy(tmp_schema_name, schema_name, schema_name_length); - tmp_schema_name[schema_name_length]='\0'; - my_casedn_str(system_charset_info, tmp_schema_name); - memcpy(ptr, tmp_schema_name, schema_name_length); - ptr+= schema_name_length; - } + ptr+= system_charset_info->casedn(schema_name, schema_name_length, + ptr, end - ptr);
That's strange. casedn of schema when lower_case_table_names==0 ?
I'm not strong with this part of the code. So here I just tried to preserve the behavior before the change. Can you suggest an SQL script which would likely to return a wrong result because of of this code?
something like this: create database FOO; create database foo; create procedure FOO.sp () select 1; create procedure foo.sp () select 2; call FOO.sp(); call foo.sp(); select object_type, object_schema, object_name, count_star, count_statements, sum_rows_sent from performance_schema.events_statements_summary_by_program where object_type='procedure'; drop database FOO; drop database foo; Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org