[MariaDB developers] Re: 2821cf816a4: MDEV-31531 Remove my_casedn_str() and my_caseup_str()

Hi, Alexander,

commit 2821cf816a4 Author: Alexander Barkov Date: Fri Jun 23 13:24:02 2023 +0400

MDEV-31531 Remove my_casedn_str() and my_caseup_str()

please, explain here why

diff --git a/include/m_ctype.h b/include/m_ctype.h index 9d13989d1fe..d2629121cfe 100644 --- a/include/m_ctype.h +++ b/include/m_ctype.h @@ -848,6 +844,26 @@ struct charset_info_st return (cset->casedn)(this, src, srclen, dst, dstlen); }

+ /* Convert to a lower-cased 0-terminated string */ + size_t casedn_z(const char *src, size_t srclen, + char *dst, size_t dstlen) const + { + DBUG_ASSERT(dstlen);

DBUG_ASSERT(src != dst) ?

+ size_t len= casedn(src, srclen, dst, dstlen - 1); + dst[len]= '\0'; + return len; + } + + /* Convert to a upper-cased 0-terminated string */ + size_t caseup_z(const char *src, size_t srclen, + char *dst, size_t dstlen) const + { + DBUG_ASSERT(dstlen); + size_t len= caseup(src, srclen, dst, dstlen - 1); + dst[len]= '\0'; + return len; + } + uint caseup_multiply() const { return (cset->caseup_multiply)(this); @@ -1512,6 +1528,14 @@ size_t my_copy_fix_mb(CHARSET_INFO *cs, /* Functions for 8bit */ extern size_t my_caseup_str_8bit(CHARSET_INFO *, char *); extern size_t my_casedn_str_8bit(CHARSET_INFO *, char *); +static inline size_t my_caseup_str_latin1(char *str) +{ + return my_caseup_str_8bit(&my_charset_latin1, str); +} +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

extern size_t my_caseup_8bit(CHARSET_INFO *, const char *src, size_t srclen, char *dst, size_t dstlen); diff --git a/sql/char_buffer.h b/sql/char_buffer.h index 7b855f5401c..1091775259d 100644 --- a/sql/char_buffer.h +++ b/sql/char_buffer.h @@ -71,25 +76,92 @@ class CharBuffer m_buff[m_length]= '\0'; return *this; } + CharBuffer & copy_caseup(CHARSET_INFO *cs, const LEX_CSTRING &str) + { + DBUG_ASSERT(!buffer_overlaps(str)); + m_length= cs->cset->caseup(cs, str.str, str.length, m_buff, buff_sz); + DBUG_ASSERT(is_sane()); + m_buff[m_length]= '\0'; // See comments in copy_casedn() + return *this; + } CharBuffer & copy_casedn(CHARSET_INFO *cs, const LEX_CSTRING &str, bool casedn) { casedn ? copy_casedn(cs, str) : copy_bin(str); return *this; } + + // Append one character + CharBuffer & append_char(char ch) + { + DBUG_ASSERT(is_sane()); + if (available_size()) + { + m_buff[m_length++]= ch; + m_buff[m_length]= '\0'; + } + DBUG_ASSERT(is_sane()); + return *this; + } + + // Append a string + CharBuffer & append_bin(const LEX_CSTRING &str)

why _bin ? and why even have a suffix, and not just append() here and above?

+ { + DBUG_ASSERT(is_sane()); + DBUG_ASSERT(!buffer_overlaps(str)); + size_t len= MY_MIN(available_size(), str.length); + memcpy(m_buff + m_length, str.str, len); + m_length+= len; + DBUG_ASSERT(is_sane()); + m_buff[m_length]= '\0'; + return *this; + } + // Append a string with casedn conversion CharBuffer & append_casedn(CHARSET_INFO *cs, const LEX_CSTRING &str) { DBUG_ASSERT(is_sane()); DBUG_ASSERT(!buffer_overlaps(str)); size_t casedn_length= cs->casedn(str.str, str.length, - m_buff + m_length, buff_sz - m_length); + m_buff + m_length, available_size()); + m_length+= casedn_length; + DBUG_ASSERT(is_sane()); + m_buff[m_length]= '\0'; + return *this; + } + + CharBuffer & append_opt_casedn(CHARSET_INFO *cs, + const LEX_CSTRING &str, + bool casedn) + { + return casedn ? append_casedn(cs, str) : append_bin(str); + } + + // Append a string with caseup conversion + CharBuffer & append_caseup(CHARSET_INFO *cs, const LEX_CSTRING &str) + { + DBUG_ASSERT(is_sane()); + DBUG_ASSERT(!buffer_overlaps(str)); + size_t casedn_length= cs->caseup(str.str, str.length, + m_buff + m_length, available_size()); m_length+= casedn_length; DBUG_ASSERT(is_sane()); m_buff[m_length]= '\0'; return *this; }

+ CharBuffer & truncate(size_t length) + { + DBUG_ASSERT(is_sane()); + if (m_length > length) + { + m_length= length; + m_buff[m_length]= '\0'; + DBUG_ASSERT(is_sane()); + } + return *this; + } + LEX_CSTRING to_lex_cstring() const { return LEX_CSTRING{m_buff, m_length}; diff --git a/sql/item_sum.cc b/sql/item_sum.cc index bbd09a59267..824ed42ec91 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -3769,20 +3769,15 @@ int group_concat_key_cmp_with_order_with_nulls(void *arg, const void *key1_arg, }

-static void report_cut_value_error(THD *thd, uint row_count, const char *fname) +static void report_cut_value_error(THD *thd, uint row_count, + const LEX_CSTRING &fname) { - size_t fn_len= strlen(fname); - char *fname_upper= (char *) my_alloca(fn_len + 1); - if (!fname_upper) - fname_upper= (char*) fname; // Out of memory - else - memcpy(fname_upper, fname, fn_len+1); - my_caseup_str(&my_charset_latin1, fname_upper); + CharBuffer fname_upper; + fname_upper.copy_caseup(&my_charset_latin1, fname);

why bother? we don't do it in other error cases

push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, ER_CUT_VALUE_GROUP_CONCAT, ER_THD(thd, ER_CUT_VALUE_GROUP_CONCAT), - row_count, fname_upper); + row_count, fname_upper.ptr()); - my_afree(fname_upper); }

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 @@ -45,6 +46,10 @@ class Lex_ident_fs: public LEX_CSTRING bool is_in_lower_case() const; bool ok_for_lower_case_names() const; #endif + bool check_db_name_quick() const

please, use a name that makes it clear what the return value is Not if (str.check_db_name_quick()) but if (str.is_invalid_db_name())

+ { + return !length || length > NAME_LEN || str[length-1] == ' '; + } };

@@ -160,4 +171,95 @@ class DBNameBuffer: public CharBuffer };

+class Identifier_chain2 +{ + LEX_CSTRING m_name[2]; +public: + Identifier_chain2() + :m_name{Lex_cstring(), Lex_cstring()} + { } + Identifier_chain2(const LEX_CSTRING &a, const LEX_CSTRING &b) + :m_name{a, b} + { } + + const LEX_CSTRING& operator [] (size_t i) const + { + return m_name[i]; + } + + static Identifier_chain2 split(const LEX_CSTRING &txt) + { + DBUG_ASSERT(txt.str[txt.length] == '\0'); // Expect 0-terminated input + const char *dot= strchr(txt.str, '.'); + if (!dot) + return Identifier_chain2(Lex_cstring(), txt); + size_t length0= dot - txt.str; + Lex_cstring name0(txt.str, length0); + Lex_cstring name1(txt.str + length0 + 1, txt.length - length0 - 1); + return Identifier_chain2(name0, name1); + } + + // Export as a qualified name string: 'db.name' + size_t make_qname(char *dst, size_t dstlen) const + { + return my_snprintf(dst, dstlen, "%.*s.%.*s", + (int) m_name[0].length, m_name[0].str, + (int) m_name[1].length, m_name[1].str); + } + + // Export as a qualified name string, allocate on mem_root. + bool make_qname(MEM_ROOT *mem_root, LEX_CSTRING *dst) const + { + const uint dot= !!m_name[0].length; + char *tmp; + /* format: [pkg + dot] + name + '\0' */ + dst->length= m_name[0].length + dot + m_name[1].length; + if (unlikely(!(dst->str= tmp= (char*) alloc_root(mem_root, + dst->length + 1)))) + return true; + snprintf(tmp, dst->length + 1, "%.*s%.*s%.*s", + (int) m_name[0].length, (m_name[0].length ? m_name[0].str : ""), + dot, ".", + (int) m_name[1].length, m_name[1].str); + return false; + } + + /* + Build a separated two step name, e.g. "ident1/ident2", 0-terminated. + with an optional lower-case conversion.

may be should be inside InnoDB?

+ @param [OUT] dst - the destination + @param dst_size - number of bytes available in dst + @param sep - the separator character + @param casedn - whether to convert components to lower case + @return - number of bytes written to "dst", not counting + the trailing '\0' byte. + */ + size_t make_sep_name_opt_casedn(char *dst, size_t dst_size, + int sep, bool casedn) const + { + /* + Minimal possible buffer is 4 bytes: 'd/t\0' + where 'd' is the database name, 't' is the table name. + Callers should never pass a smaller buffer. + */ + DBUG_ASSERT(dst_size > 4); + DBUG_ASSERT(m_name[0].length + m_name[1].length + 2 < FN_REFLEN - 1); + if (casedn) + { + CHARSET_INFO *cs= &my_charset_utf8mb3_general_ci; + char *ptr= dst, *end= dst + dst_size; + ptr+= cs->casedn(m_name[0].str, m_name[0].length, ptr, end - ptr - 2); + *ptr++= (char) sep; + ptr+= cs->casedn_z(m_name[1].str, m_name[1].length, ptr, end - ptr); + return ptr - dst; + } + memcpy(dst, m_name[0].str, m_name[0].length); + dst[m_name[0].length] = (char) sep; + /* Copy the name and null-byte. */ + memcpy(dst + m_name[0].length + 1, m_name[1].str, m_name[1].length + 1); + return m_name[0].length + 1/*slash*/ + m_name[1].length;

s/slash/sep/

+ } +}; + + #endif // LEX_IDENT_INCLUDED diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 6030202adc7..0fc1d4ce962 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -5777,19 +5778,29 @@ int Table_map_log_event::do_apply_event(rpl_group_info *rgi) /* Step the query id to mark what columns that are actually used. */ thd->set_query_id(next_query_id());

+ db_mem_alloced= tname_mem_alloced= NAME_LEN * + (lower_case_table_names ? 1 : + files_charset_info->casedn_multiply()) + + 1 /* for '\0' */;

1. may be db_mem_length/tname_mem_length instead of NAME_LEN? 2. is the ?: condition right? Looks like it's inverted

if (!(memory= my_multi_malloc(PSI_INSTRUMENT_ME, MYF(MY_WME), &table_list, (uint) sizeof(RPL_TABLE_LIST), - &db_mem, (uint) NAME_LEN + 1, - &tname_mem, (uint) NAME_LEN + 1, + &db_mem, (uint) db_mem_alloced, + &tname_mem, (uint) tname_mem_alloced, NullS))) DBUG_RETURN(HA_ERR_OUT_OF_MEM);

- db_mem_length= strmov(db_mem, m_dbnam) - db_mem; - tname_mem_length= strmov(tname_mem, m_tblnam) - tname_mem; if (lower_case_table_names) { - my_casedn_str(files_charset_info, (char*)tname_mem); - my_casedn_str(files_charset_info, (char*)db_mem); + db_mem_length= files_charset_info->casedn_z(m_dbnam, m_dblen, + db_mem, db_mem_alloced); + tname_mem_length= files_charset_info->casedn_z(m_tblnam, m_tbllen, + tname_mem, + tname_mem_alloced); + } + else + { + db_mem_length= strmov(db_mem, m_dbnam) - db_mem; + tname_mem_length= strmov(tname_mem, m_tblnam) - tname_mem; }

/* call from mysql_client_binlog_statement() will not set rli->mi */ diff --git a/sql/sp.cc b/sql/sp.cc index 0a8aa24789b..537d29fb67e 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -2281,12 +2281,20 @@ Sp_handler::sp_exist_routines(THD *thd, TABLE_LIST *routines) const for (routine= routines; routine; routine= routine->next_global) { sp_name *name; - LEX_CSTRING lex_db; + LEX_CSTRING lex_db= thd->make_ident_opt_casedn(routine->db, + lower_case_table_names); + if (!lex_db.str) + DBUG_RETURN(TRUE); // EOM

// EOM, error was already sent right? (also, in other places)

LEX_CSTRING lex_name; - thd->make_lex_string(&lex_db, routine->db.str, routine->db.length); thd->make_lex_string(&lex_name, routine->table_name.str, routine->table_name.length);

Rather inconsistent :( May be, makes sense to create LEX_CSTRING make_lex_string(const char* str, size_t length) const ?

+ /* + routine->db was earlier tested with check_db_name(). + Now it's lower-cased according to lower_case_table_names. + It's safe to make a Lex_ident_db_normalized. + */ - name= new sp_name(&lex_db, &lex_name, true); + name= new (thd->mem_root) sp_name(Lex_ident_db_normalized(lex_db), + lex_name, true); sp_object_found= sp_find_routine(thd, name, false) != NULL; thd->get_stmt_da()->clear_warning_info(thd->query_id); if (! sp_object_found) diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 09af5523c76..70dde7dc0ed 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2543,6 +2543,37 @@ static void push_new_user(const ACL_USER &user) }

+/** + Make a database name on mem_root from a String, + apply lower-case conversion if lower_case_table_names says so. + Perform database name length limit validation. + + @param thd - the THD, to get the warning text from + @param mem_root - allocate the result on this memory root + @param dbstr - the String, e.g. with Field::val_str() result + + @return - {NULL,0} in case of EOM or a bad database name, + or a good database name otherwise. +*/ +static LEX_STRING make_and_check_db_name(THD *thd, MEM_ROOT *mem_root, + const String &dbstr) +{ + LEX_STRING dbls= lower_case_table_names ? + lex_string_casedn_root(mem_root, files_charset_info, + dbstr.ptr(), dbstr.length()) : + lex_string_strmake_root(mem_root, + dbstr.ptr(), dbstr.length()); + if (!dbls.str) + return LEX_STRING{NULL, 0}; // EOM + 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?

+ return LEX_STRING{NULL, 0}; // Bad name + } + return dbls; // Good name +} + + /* Initialize structures responsible for user/db-level privilege checking and load information about grants from open privilege tables. @@ -3680,26 +3702,27 @@ privilege_t acl_get(const char *host, const char *ip, { privilege_t host_access(ALL_KNOWN_ACL), db_access(NO_ACL); uint i; - size_t key_length; - char key[ACL_KEY_LENGTH],*tmp_db,*end; + const char *tmp_db; acl_entry *entry; DBUG_ENTER("acl_get");

- tmp_db= strmov(strmov(key, safe_str(ip)) + 1, user) + 1; - end= strnmov(tmp_db, db, key + sizeof(key) - tmp_db); - - if (end >= key + sizeof(key)) // db name was truncated + constexpr size_t key_data_size= ACL_KEY_LENGTH + 2; + CharBuffer key;

why ACL_KEY_LENGTH + 2 + MY_CS_MBMAXLEN ?

+ key.append_bin(Lex_cstring_strlen(safe_str(ip))); + key.append_char('\0'); + key.append_bin(Lex_cstring_strlen(user)); + key.append_char('\0');

there was strmov(strmov(...)) pattern here before. as far as I can see, you can chain too: key.append_bin().append_char() .append_bin().append_char(); let's use that

+ tmp_db= key.end(); + key.append_opt_casedn(files_charset_info, Lex_cstring_strlen(db), + lower_case_table_names); + db= tmp_db; + + if (key.length() >= key_data_size) // db name was truncated DBUG_RETURN(NO_ACL); // no privileges for an invalid db name

- if (lower_case_table_names) - { - my_casedn_str(files_charset_info, tmp_db); - db=tmp_db; - } - key_length= (size_t) (end-key); - mysql_mutex_lock(&acl_cache->lock); - if (!db_is_pattern && (entry=acl_cache->search((uchar*) key, key_length))) + if (!db_is_pattern && + (entry= acl_cache->search((uchar*) key.ptr(), key.length()))) { db_access=entry->access; mysql_mutex_unlock(&acl_cache->lock); @@ -5630,28 +5662,26 @@ static GRANT_NAME *name_hash_search(HASH *name_hash, const char *user, const char *tname, bool exact, bool name_tolower) { - char helping[SAFE_NAME_LEN*2+USERNAME_LENGTH+3]; - char *hend = helping + sizeof(helping); - uint len; + constexpr size_t key_data_size= SAFE_NAME_LEN * 2 + USERNAME_LENGTH + 3; + CharBuffer key; GRANT_NAME *grant_name,*found=0; HASH_SEARCH_STATE state;

- char *db_ptr= strmov(helping, user) + 1; - char *tname_ptr= strnmov(db_ptr, db, hend - db_ptr) + 1; - if (tname_ptr > hend) - return 0; // invalid name = not found - char *end= strnmov(tname_ptr, tname, hend - tname_ptr) + 1; - if (end > hend) + key.append_bin(Lex_cstring_strlen(user)); + key.append_char('\0'); + key.append_bin(Lex_cstring_strlen(db));

is db already normalized? If yes, then, please, document it with DBUG_ASSERT(ok_for_lower_case_names());

+ key.append_char('\0'); + key.append_opt_casedn(files_charset_info, Lex_cstring_strlen(tname), + name_tolower); + key.append_char('\0'); + if (key.length() >= key_data_size) return 0; // invalid name = not found

- len = (uint) (end - helping); - if (name_tolower) - my_casedn_str(files_charset_info, tname_ptr); - for (grant_name= (GRANT_NAME*) my_hash_first(name_hash, (uchar*) helping, - len, &state); + for (grant_name= (GRANT_NAME*) my_hash_first(name_hash, (uchar*) key.ptr(), + key.length(), &state); grant_name ; - grant_name= (GRANT_NAME*) my_hash_next(name_hash,(uchar*) helping, - len, &state)) + grant_name= (GRANT_NAME*) my_hash_next(name_hash, (uchar*) key.ptr(), + key.length(), &state)) { if (exact) { @@ -8826,33 +8856,28 @@ static bool check_grant_db_routine(THD *thd, const char *db, HASH *hash) bool check_grant_db(THD *thd, const char *db) { Security_context *sctx= thd->security_ctx; - char helping [SAFE_NAME_LEN + USERNAME_LENGTH+2], *end; - char helping2 [SAFE_NAME_LEN + USERNAME_LENGTH+2], *tmp_db; - uint len, UNINIT_VAR(len2); + constexpr size_t key_data_size= SAFE_NAME_LEN + USERNAME_LENGTH + 2; + CharBuffer key, key2; bool error= TRUE;

- tmp_db= strmov(helping, sctx->priv_user) + 1; - end= strnmov(tmp_db, db, helping + sizeof(helping) - tmp_db); - - if (end >= helping + sizeof(helping)) // db name was truncated - return 1; // no privileges for an invalid db name - - if (lower_case_table_names) - { - end = tmp_db + my_casedn_str(files_charset_info, tmp_db); - db=tmp_db; - } + key.append_bin(Lex_cstring_strlen(sctx->priv_user)); + key.append_char('\0'); + key.append_opt_casedn(files_charset_info, Lex_cstring_strlen(db), + lower_case_table_names); + key.append_char('\0');

- len= (uint) (end - helping) + 1; + if (key.length() >= key_data_size) // db name was truncated + return 1; // no privileges for an invalid db name

/* If a role is set, we need to check for privileges here as well. */ if (sctx->priv_role[0]) { - end= strmov(helping2, sctx->priv_role) + 1; - end= strnmov(end, db, helping2 + sizeof(helping2) - end); - len2= (uint) (end - helping2) + 1; + key2.append_bin(Lex_cstring_strlen(sctx->priv_role)); + key2.append_char('\0'); + key2.append_bin(Lex_cstring_strlen(db));

No append_opt_casedn() here? Looks wrong.

+ key2.append_char('\0'); }

diff --git a/sql/sql_class.h b/sql/sql_class.h index 214118d8431..decb9a9064e 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -4918,11 +4948,46 @@ class THD: public THD_count, /* this must be first */ my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); return TRUE; } + return false; + } + + /* + Copy the current database to the argument. Use the current arena to + allocate memory for a deep copy: current database may be freed after + a statement is parsed but before it's executed. + + Can only be called by owner of thd (no mutex protection) + */ + bool copy_db_to(LEX_CSTRING *to) + { + if (check_if_current_db_is_set_with_error()) + return true;

to->str= strmake(db.str, db.length); to->length= db.length; return to->str == NULL; /* True on error */ } + + /* + Make a normalized copy of the current database. + Raise an error if no current database is set. + Note, in case of lower_case_table_names==2, thd->db can contain the + name in arbitrary case typed by the user, so it must be lower-cased. + For other lower_case_table_names values the name is already in + its normalized case, so it's copied as is. + */ + Lex_ident_db_normalized copy_db_normalized() + { + if (check_if_current_db_is_set_with_error()) + return Lex_ident_db_normalized(); + LEX_STRING ident= make_ident_opt_casedn(db, lower_case_table_names == 2);

LEX_STRING or LEX_CSTRING ?

+ /* + A non-empty thd->db is always known to satisfy check_db_name(). + So after optional lower-casing above it's safe to + make Lex_ident_db_normalized. + */ + return Lex_ident_db_normalized(ident); + } /* Get db name or "". Use for printing current db */ const char *get_db() { return safe_str(db.str); } diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index a84fd29affc..8cafad65538 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -4186,6 +4186,18 @@ bool LEX::copy_db_to(LEX_CSTRING *to) return thd->copy_db_to(to); }

+ +Lex_ident_db_normalized LEX::copy_db_normalized() +{ + if (sphead && sphead->m_name.str) + { + DBUG_ASSERT(sphead->m_db.str && sphead->m_db.length);

Please, DBUG_ASSERT(sphead->m_db.str); DBUG_ASSERT(sphead->m_db.length);

+ return thd->to_ident_db_normalized_with_error(sphead->m_db); + } + return thd->copy_db_normalized(); +} + + /** Initialize offset and limit counters.

diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 1fcafd6ec08..0f39a13632a 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -3856,30 +3859,31 @@ static bool show_status_array(THD *thd, const char *wild, names until lp:1306875 has been fixed. TODO: remove once lp:1306875 has been addressed. */ - if (!(*prefix) && !strncasecmp(name_buffer, "wsrep", strlen("wsrep"))) + if (!prefix.length && + !strncasecmp(name_buffer.ptr(), "wsrep", strlen("wsrep"))) { is_wsrep_var= TRUE; } #endif /* WITH_WSREP */

if (ucase_names) - my_caseup_str(system_charset_info, name_buffer); + name_buffer.append_caseup(system_charset_info, var_name); else { - my_casedn_str(system_charset_info, name_buffer); - DBUG_ASSERT(name_buffer[0] >= 'a'); - DBUG_ASSERT(name_buffer[0] <= 'z'); - + name_buffer.append_casedn(system_charset_info, var_name); // WSREP_TODO: remove once lp:1306875 has been addressed. if (IF_WSREP(is_wsrep_var == FALSE, 1) && status_var) - name_buffer[0]-= 'a' - 'A'; + { + char *ptr= (char*) name_buffer.ptr(); + if (ptr[0] >= 'a' && ptr[0] <= 'z') + ptr[0]-= 'a' - 'A'; + }

is it still needed?

}

restore_record(table, s->default_values); - table->field[0]->store(name_buffer, strlen(name_buffer), - system_charset_info); + table->field[0]->store(name_buffer.to_lex_cstring(), system_charset_info);

/* Compare name for types that can't return arrays. We do this to not @@ -4355,18 +4361,15 @@ bool get_lookup_field_values(THD *thd, COND *cond, bool fix_table_name_case,

if (lower_case_table_names && !rc) { - /* - We can safely do in-place upgrades here since all of the above cases - are allocating a new memory buffer for these strings. - */ if (lookup_field_values->db_value.str && lookup_field_values->db_value.str[0]) - my_casedn_str(system_charset_info, - (char*) lookup_field_values->db_value.str); + lookup_field_values->db_value= thd->make_ident_casedn( + lookup_field_values->db_value); + if (fix_table_name_case && lookup_field_values->table_value.str && lookup_field_values->table_value.str[0]) - my_casedn_str(system_charset_info, - (char*) lookup_field_values->table_value.str); + lookup_field_values->table_value= thd->make_ident_casedn( + lookup_field_values->table_value);

Why is it even needed? If db/table names are compared case insensitively, it shouldn't matter if they're lowercased or not, should it?

}

return rc; diff --git a/sql/sql_type_fixedbin.h b/sql/sql_type_fixedbin.h index 8aaa7582cd5..e9728b78a3d 100644 --- a/sql/sql_type_fixedbin.h +++ b/sql/sql_type_fixedbin.h @@ -267,8 +267,7 @@ class Type_handler_fbt: public Type_handler void print(String *str, enum_query_type query_type) override { StringBufferFbtImpl::max_char_length()+64 tmp; - tmp.append(singleton()->name().lex_cstring()); - my_caseup_str(&my_charset_latin1, tmp.c_ptr()); + tmp.copy_caseup(&my_charset_latin1, singleton()->name().lex_cstring());

why bother?

str->append(tmp); str->append('\''); m_value.to_string(&tmp); diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/dict0load.cc index bf59fed9ee4..104a8dc8780 100644 --- a/storage/innobase/dict/dict0load.cc +++ b/storage/innobase/dict/dict0load.cc @@ -2955,7 +2955,7 @@ dict_load_foreign(

foreign->foreign_table_name = mem_heap_strdupl( foreign->heap, (char*) field, len); - dict_mem_foreign_table_name_lookup_set(foreign, TRUE); + foreign->foreign_table_name_lookup_set();

please, ask Marko to review InnoDB changes

const size_t foreign_table_name_len = len; const size_t table_name_len = strlen(table_name); diff --git a/storage/perfschema/pfs_instr_class.h b/storage/perfschema/pfs_instr_class.h index f353c410d4c..d083db63968 100644 --- a/storage/perfschema/pfs_instr_class.h +++ b/storage/perfschema/pfs_instr_class.h @@ -265,6 +265,40 @@ struct PFS_table_share_key char m_hash_key[PFS_TABLESHARE_HASHKEY_SIZE]; /** Length in bytes of @c m_hash_key. */ uint m_key_length; + + size_t available_length() const + { + return sizeof(m_hash_key) - m_key_length; + } + char *end() + { + return m_hash_key + m_key_length; + } + // Append and 0-terminate a string with an optional lower-case conversion + void append_opt_casedn_z(CHARSET_INFO *cs, + const char *str, size_t length, + bool casedn) + { + DBUG_ASSERT(length <= sizeof(m_hash_key)); // Expect valid db/tbl names + MY_STRCOPY_STATUS st; + size_t dst_length= available_length(); + if (dst_length > 0) + { + dst_length--; + /* + The code below uses copy_fix() instead of memcpy() to make + sure we don't break a multi-byte character in the middle. + */ + m_key_length+= (uint) (casedn ? + cs->casedn(str, length, end(), dst_length) : + cs->copy_fix(end(), dst_length, + str, length, length, &st));

may be let's not do copy_fix here? there are table names and in a perfschema, for PFS_table_share_key. memcpy would be enough.

+ m_hash_key[m_key_length++]= '\0'; + } + } + void set(bool temporary, + const char *schema_name, size_t schema_name_length, + const char *table_name, size_t table_name_length); };

/** Table index or 'key' */ 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 ?

ptr[0]= 0; ptr++;

Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org