Skip to content

Commit

Permalink
MDEV-31606 Refactor check_db_name() to get a const argument
Browse files Browse the repository at this point in the history
Problem:
Under terms of MDEV-27490, we'll update Unicode version used
to compare identifiers to 14.0.0. Unlike in the old Unicode version,
in the new version a string can grow during lower-case. We cannot
perform check_db_name() inplace any more.

Change summary:

- Allocate memory to store lower-cased identifiers in memory root

- Removing check_db_name() performing both in-place lower-casing and validation
  at the same time. Splitting it into two separate stages:
  * creating a memory-root lower-cased copy of an identifier
    (using new MEM_ROOT functions and Query_arena wrapper methods)
  * performing validation on a constant string
    (using Lex_ident_fs methods)

Implementation details:

- Adding a mysys helper function to allocate lower-cased strings on MEM_ROOT:

    lex_string_casedn_root()

  and a Query_arena wrappers for it:

    make_ident_casedn()
    make_ident_opt_casedn()

- Adding a Query_arena method to perform both MEM_ROOT lower-casing and
  database name validation at the same time:

    to_ident_db_internal_with_error()

  This method is very close to the old (pre-11.3) check_db_name(),
  but performs lower-casing to a newly allocated MEM_ROOT
  memory (instead of performing lower-casing the original string in-place).

- Adding a Table_ident method which additionally handles derived table names:

    to_ident_db_internal_with_error()

- Removing the old check_db_name()
  • Loading branch information
abarkov committed Sep 13, 2023
1 parent e987b93 commit f5aae71
Show file tree
Hide file tree
Showing 15 changed files with 244 additions and 172 deletions.
14 changes: 14 additions & 0 deletions include/my_sys.h
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,20 @@ static inline char *safe_strdup_root(MEM_ROOT *root, const char *str)
extern char *strmake_root(MEM_ROOT *root,const char *str,size_t len);
extern void *memdup_root(MEM_ROOT *root,const void *str, size_t len);
extern LEX_CSTRING safe_lexcstrdup_root(MEM_ROOT *root, const LEX_CSTRING str);

static inline LEX_STRING lex_string_strmake_root(MEM_ROOT *mem_root,
const char *str, size_t length)
{
LEX_STRING tmp;
tmp.str= strmake_root(mem_root, str, length);
tmp.length= tmp.str ? length : 0;
return tmp;
}

extern LEX_STRING lex_string_casedn_root(MEM_ROOT *root,
CHARSET_INFO *cs,
const char *str, size_t length);

extern my_bool my_compress(uchar *, size_t *, size_t *);
extern my_bool my_uncompress(uchar *, size_t , size_t *);
extern uchar *my_compress_alloc(const uchar *packet, size_t *len,
Expand Down
13 changes: 13 additions & 0 deletions mysys/my_alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -623,3 +623,16 @@ LEX_CSTRING safe_lexcstrdup_root(MEM_ROOT *root, const LEX_CSTRING str)
res.length= str.length;
return res;
}


LEX_STRING lex_string_casedn_root(MEM_ROOT *root, CHARSET_INFO *cs,
const char *str, size_t length)
{
size_t nbytes= length * cs->cset->casedn_multiply(cs);
LEX_STRING res= {NULL, 0};
if (!(res.str= alloc_root(root, nbytes + 1)))
return res;
res.length= cs->cset->casedn(cs, str, length, res.str, nbytes);
res.str[res.length]= '\0';
return res;
}
8 changes: 7 additions & 1 deletion sql/lex_ident.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class Lex_ident_fs: public LEX_CSTRING
bool check_db_name() const;
bool check_db_name_with_error() const;
#ifndef DBUG_OFF
bool is_in_lower_case() const;
bool ok_for_lower_case_names() const;
#endif
};
Expand All @@ -55,14 +56,19 @@ class Lex_ident_fs: public LEX_CSTRING
*/
class Lex_ident_db: public Lex_ident_fs
{
// {empty_c_string,0} is used by derived tables
bool is_empty() const
{
return length == 0 && str != NULL;
}
public:
Lex_ident_db()
:Lex_ident_fs(NULL, 0)
{ }
Lex_ident_db(const char *str, size_t length)
:Lex_ident_fs(str, length)
{
DBUG_SLOW_ASSERT(!check_db_name());
DBUG_SLOW_ASSERT(is_empty() || !check_db_name());
}
};

Expand Down
4 changes: 2 additions & 2 deletions sql/sp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2620,9 +2620,9 @@ bool Sp_handler::
pkgstr, name->m_name, type()))
{
DBUG_ASSERT(ret == SP_OK);
pkgname->copy(thd->mem_root, caller->m_db, pkgstr);
*pkg_routine_handler= package_routine_handler();
if (name->make_package_routine_name(thd->mem_root, pkgstr, name->m_name))
if (pkgname->copy_sp_name_internal(thd->mem_root, caller->m_db, pkgstr) ||
name->make_package_routine_name(thd->mem_root, pkgstr, name->m_name))
return true;
}
return ret != SP_OK;
Expand Down
8 changes: 4 additions & 4 deletions sql/sp_head.cc
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ sp_head::init(LEX *lex)
}


void
bool
sp_head::init_sp_name(const sp_name *spname)
{
DBUG_ENTER("sp_head::init_sp_name");
Expand All @@ -819,10 +819,10 @@ sp_head::init_sp_name(const sp_name *spname)

DBUG_ASSERT(spname && spname->m_db.str && spname->m_db.length);

/* We have to copy strings to get them into the right memroot. */
Database_qualified_name::copy(&main_mem_root, spname->m_db, spname->m_name);
m_explicit_name= spname->m_explicit_name;
DBUG_VOID_RETURN;
/* We have to copy strings to get them into the right memroot. */
DBUG_RETURN(copy_sp_name_internal(&main_mem_root,
spname->m_db, spname->m_name));
}

void
Expand Down
2 changes: 1 addition & 1 deletion sql/sp_head.h
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ class sp_head :private Query_arena,
init(LEX *lex);

/** Copy sp name from parser. */
void
bool
init_sp_name(const sp_name *spname);

/** Set the body-definition start position. */
Expand Down
61 changes: 54 additions & 7 deletions sql/sql_class.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3975,6 +3975,49 @@ Query_arena::Type Statement::type() const
}


/*
Return an internal database name:
- validated with Lex_ident_db::check_db_name()
- optionally converted to lower-case when lower_case_table_names==1
The lower-cased copy is made on mem_root when needed.
An error is raised in case of EOM or a bad database name.
@param src - the database name
@returns - {NULL,0} on EOM or a bad database name,
or a good database name otherwise
*/

Lex_ident_db
Query_arena::to_ident_db_internal_with_error(const LEX_CSTRING &src)
{
DBUG_ASSERT(src.str);
if (src.str == any_db.str) // e.g. JSON table
return any_db; // preserve any_db - it has a special meaning

bool casedn= lower_case_table_names == 1;
const LEX_CSTRING tmp= casedn ? make_ident_casedn(src) : src;
if (!tmp.str /*EOM*/ ||
Lex_ident_fs(tmp).check_db_name_with_error())
return Lex_ident_db();

return Lex_ident_db(tmp.str, tmp.length);
}


Lex_ident_db
Table_ident::to_ident_db_internal_with_error(Query_arena *arena) const
{
if (is_derived_table())
{
DBUG_ASSERT(db.str == empty_c_string && db.length == 0);
return Lex_ident_db(empty_c_string, 0);
}
// Normal table or JSON table
return arena->to_ident_db_internal_with_error(db);
}


void Statement::set_statement(Statement *stmt)
{
id= stmt->id;
Expand Down Expand Up @@ -8168,14 +8211,18 @@ void AUTHID::parse(const char *str, size_t length)
}


void Database_qualified_name::copy(MEM_ROOT *mem_root,
const LEX_CSTRING &db,
const LEX_CSTRING &name)
bool Database_qualified_name::copy_sp_name_internal(MEM_ROOT *mem_root,
const LEX_CSTRING &db,
const LEX_CSTRING &name)
{
m_db.length= db.length;
m_db.str= strmake_root(mem_root, db.str, db.length);
m_name.length= name.length;
m_name.str= strmake_root(mem_root, name.str, name.length);
DBUG_ASSERT(db.str);
DBUG_ASSERT(name.str);
m_db= lower_case_table_names == 1 ?
lex_string_casedn_root(mem_root, &my_charset_utf8mb3_general_ci,
db.str, db.length) :
lex_string_strmake_root(mem_root, db.str, db.length);
m_name= lex_string_strmake_root(mem_root, name.str, name.length);
return m_db.str == NULL || m_name.str == NULL; // check if EOM
}


Expand Down
58 changes: 56 additions & 2 deletions sql/sql_class.h
Original file line number Diff line number Diff line change
Expand Up @@ -1355,6 +1355,45 @@ class Query_arena
return false;
}

/*
Make a lower-cased copy of an identifier on mem_root.

@param src - The original identifier (usually coming from the parser)
@return - {NULL,0} in case of EOM, or a non-NULL LEX_STRING
with the lower-cased identifier copy.
*/
LEX_STRING make_ident_casedn(const LEX_CSTRING &src)
{
return lex_string_casedn_root(mem_root, &my_charset_utf8mb3_general_ci,
src.str, src.length);
}

/*
Make an exact copy or a lower-cased copy of an identifier on mem_root.

@param src - The original identifier (usually coming from the parser)
@param casedn - If the name should be converted to lower case
@return - {NULL,0} in case of EOM,
or a non-NULL LEX_STRING with the identifier copy.
*/
LEX_STRING make_ident_opt_casedn(const LEX_CSTRING &src, bool casedn)
{
return casedn ? make_ident_casedn(src) :
lex_string_strmake_root(mem_root, src.str, src.length);
}

/*
Convert a LEX_CSTRING to a valid internal database name:
- validated with Lex_ident_fs::check_db_name()
- optionally lower-cased when lower_case_table_names==1
The lower-cased copy is created on Query_arena::mem_root, when needed.

@param name - The name to normalize. Must not be {NULL,0}.
@return - {NULL,0} on EOM or a bad database name
(with an errror is raised,
or a good database name otherwise.
*/
Lex_ident_db to_ident_db_internal_with_error(const LEX_CSTRING &name);

void set_query_arena(Query_arena *set);

Expand Down Expand Up @@ -7051,6 +7090,16 @@ class Table_ident :public Sql_alloc
}
bool resolve_table_rowtype_ref(THD *thd, Row_definition_list &defs);
bool append_to(THD *thd, String *to) const;
/*
Convert Table_ident::m_db to a valid internal database name:
- validated with Lex_ident_fs::check_db_name()
- optionally lower-cased when lower_case_table_names==1

@param arena - the arena to allocate the lower-cased copy on, when needed.
@return {NULL,0} in case of EOM or invalid database name,
or a good identifier otherwise.
*/
Lex_ident_db to_ident_db_internal_with_error(Query_arena *arena) const;
};


Expand Down Expand Up @@ -7865,8 +7914,13 @@ class Database_qualified_name
!cs->strnncoll(m_name.str, m_name.length,
other->m_name.str, other->m_name.length);
}
void copy(MEM_ROOT *mem_root, const LEX_CSTRING &db,
const LEX_CSTRING &name);
/*
Make copies of "db" and "name" on the memory root in internal format:
- Lower-case "db" if lower-case-table-names==1.
- Preserve "name" as is.
*/
bool copy_sp_name_internal(MEM_ROOT *mem_root, const LEX_CSTRING &db,
const LEX_CSTRING &name);

// Export db and name as a qualified name string: 'db.name'
size_t make_qname(char *dst, size_t dstlen) const
Expand Down

0 comments on commit f5aae71

Please sign in to comment.