Hi, Alexander,
> commit 2821cf816a4
> Author: Alexander Barkov <bar(a)mariadb.com>
> 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<buff_sz> & 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<buff_sz> & 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<buff_sz> & 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<buff_sz> & 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<buff_sz> & 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<buff_sz> & 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<buff_sz> & 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<buff_sz> & 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<NAME_LEN> 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<SAFE_NAME_LEN + MY_CS_MBMAXLEN>
> };
>
>
> +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_data_size + MY_CS_MBMAXLEN> 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_data_size + MY_CS_MBMAXLEN> 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_data_size + MY_CS_MBMAXLEN> 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
> {
> StringBuffer<FbtImpl::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(a)mariadb.org
