Commit a8a75ba2 authored by Dave Gosselin's avatar Dave Gosselin Committed by Dave Gosselin

Factor TABLE_LIST creation from add_table_to_list

Ideally our methods and functions should do one thing, do that well,
and do only that.  add_table_to_list does far more than adding a
table to a list, so this commit factors the TABLE_LIST creation out
to a new TABLE_LIST constructor.  It then uses placement new()
to create it in the correct memory area (result of thd->calloc).
Benefits of this approach:
 1. add_table_to_list now returns as early as possible on an error
 2. fewer side-effects incurred on creating the TABLE_LIST object
 3. TABLE_LIST won't be calloc'd if copy_to_db fails
 4. local declarations moved closer to their respective first uses
 5. improved code readability and logical flow
Also factored a couple of other functions to keep the happy path
more to the left, which makes them easier to follow at a glance.
parent 8dda6027
...@@ -4832,8 +4832,13 @@ class THD: public THD_count, /* this must be first */ ...@@ -4832,8 +4832,13 @@ class THD: public THD_count, /* this must be first */
*/ */
bool copy_db_to(LEX_CSTRING *to) bool copy_db_to(LEX_CSTRING *to)
{ {
if (db.str == NULL) if (db.str)
{ {
to->str= strmake(db.str, db.length);
to->length= db.length;
return to->str == NULL; /* True on error */
}
/* /*
No default database is set. In this case if it's guaranteed that No default database is set. In this case if it's guaranteed that
no CTE can be used in the statement then we can throw an error right no CTE can be used in the statement then we can throw an error right
...@@ -4846,11 +4851,6 @@ class THD: public THD_count, /* this must be first */ ...@@ -4846,11 +4851,6 @@ class THD: public THD_count, /* this must be first */
my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0)); my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));
return TRUE; return TRUE;
} }
to->str= strmake(db.str, db.length);
to->length= db.length;
return to->str == NULL; /* True on error */
}
/* Get db name or "". Use for printing current db */ /* Get db name or "". Use for printing current db */
const char *get_db() const char *get_db()
{ return safe_str(db.str); } { return safe_str(db.str); }
......
...@@ -4290,17 +4290,18 @@ uint8 LEX::get_effective_with_check(TABLE_LIST *view) ...@@ -4290,17 +4290,18 @@ uint8 LEX::get_effective_with_check(TABLE_LIST *view)
bool LEX::copy_db_to(LEX_CSTRING *to) bool LEX::copy_db_to(LEX_CSTRING *to)
{ {
if (sphead && sphead->m_name.str) if (!sphead || !sphead->m_name.str)
{ return thd->copy_db_to(to);
DBUG_ASSERT(sphead->m_db.str && sphead->m_db.length);
DBUG_ASSERT(sphead->m_db.str);
DBUG_ASSERT(sphead->m_db.length);
/* /*
It is safe to assign the string by-pointer, both sphead and It is safe to assign the string by-pointer, both sphead and
its statements reside in the same memory root. its statements reside in the same memory root.
*/ */
*to= sphead->m_db; *to= sphead->m_db;
return FALSE; return FALSE;
}
return thd->copy_db_to(to);
} }
/** /**
......
...@@ -8283,10 +8283,6 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, ...@@ -8283,10 +8283,6 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
List<String> *partition_names, List<String> *partition_names,
LEX_STRING *option) LEX_STRING *option)
{ {
TABLE_LIST *ptr;
TABLE_LIST *UNINIT_VAR(previous_table_ref); /* The table preceding the current one. */
LEX_CSTRING alias_str;
LEX *lex= thd->lex;
DBUG_ENTER("add_table_to_list"); DBUG_ENTER("add_table_to_list");
DBUG_PRINT("enter", ("Table '%s' (%p) Select %p (%u)", DBUG_PRINT("enter", ("Table '%s' (%p) Select %p (%u)",
(alias ? alias->str : table->table.str), (alias ? alias->str : table->table.str),
...@@ -8296,9 +8292,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, ...@@ -8296,9 +8292,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
if (unlikely(!table)) if (unlikely(!table))
DBUG_RETURN(0); // End of memory DBUG_RETURN(0); // End of memory
alias_str= alias ? *alias : table->table; if (!(table_options & TL_OPTION_ALIAS) &&
DBUG_ASSERT(alias_str.str);
if (!MY_TEST(table_options & TL_OPTION_ALIAS) &&
unlikely(check_table_name(table->table.str, table->table.length, FALSE))) unlikely(check_table_name(table->table.str, table->table.length, FALSE)))
{ {
my_error(ER_WRONG_TABLE_NAME, MYF(0), table->table.str); my_error(ER_WRONG_TABLE_NAME, MYF(0), table->table.str);
...@@ -8313,54 +8307,21 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, ...@@ -8313,54 +8307,21 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
DBUG_RETURN(0); DBUG_RETURN(0);
} }
if (!alias) /* Alias is case sensitive */ LEX_CSTRING db{0, 0};
{ bool fqtn= false;
if (unlikely(table->sel)) LEX *lex= thd->lex;
{
my_message(ER_DERIVED_MUST_HAVE_ALIAS,
ER_THD(thd, ER_DERIVED_MUST_HAVE_ALIAS), MYF(0));
DBUG_RETURN(0);
}
/* alias_str points to table->table; Let's make a copy */
if (unlikely(!(alias_str.str= (char*) thd->memdup(alias_str.str, alias_str.length+1))))
DBUG_RETURN(0);
}
if (unlikely(!(ptr = (TABLE_LIST *) thd->calloc(sizeof(TABLE_LIST)))))
DBUG_RETURN(0); /* purecov: inspected */
if (table->db.str) if (table->db.str)
{ {
ptr->is_fqtn= TRUE; fqtn= TRUE;
ptr->db= table->db; db= table->db;
} }
else if (!lex->with_cte_resolution && lex->copy_db_to(&ptr->db)) else if (!lex->with_cte_resolution && lex->copy_db_to(&db))
DBUG_RETURN(0); DBUG_RETURN(0);
else else
ptr->is_fqtn= FALSE; fqtn= FALSE;
bool info_schema= is_infoschema_db(&db);
ptr->alias= alias_str; if (!table->sel && info_schema &&
ptr->is_alias= alias ? TRUE : FALSE; (table_options & TL_OPTION_UPDATING) &&
if (lower_case_table_names)
{
if (table->table.length)
table->table.length= my_casedn_str(files_charset_info,
(char*) table->table.str);
if (ptr->db.length && ptr->db.str != any_db.str)
ptr->db.length= my_casedn_str(files_charset_info, (char*) ptr->db.str);
}
ptr->table_name= table->table;
ptr->lock_type= lock_type;
ptr->mdl_type= mdl_type;
ptr->table_options= table_options;
ptr->updating= MY_TEST(table_options & TL_OPTION_UPDATING);
/* TODO: remove TL_OPTION_FORCE_INDEX as it looks like it's not used */
ptr->force_index= MY_TEST(table_options & TL_OPTION_FORCE_INDEX);
ptr->ignore_leaves= MY_TEST(table_options & TL_OPTION_IGNORE_LEAVES);
ptr->sequence= MY_TEST(table_options & TL_OPTION_SEQUENCE);
ptr->derived= table->sel;
if (!ptr->derived && is_infoschema_db(&ptr->db))
{
if (ptr->updating &&
/* Special cases which are processed by commands itself */ /* Special cases which are processed by commands itself */
lex->sql_command != SQLCOM_CHECK && lex->sql_command != SQLCOM_CHECK &&
lex->sql_command != SQLCOM_CHECKSUM) lex->sql_command != SQLCOM_CHECKSUM)
...@@ -8371,19 +8332,30 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, ...@@ -8371,19 +8332,30 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
INFORMATION_SCHEMA_NAME.str); INFORMATION_SCHEMA_NAME.str);
DBUG_RETURN(0); DBUG_RETURN(0);
} }
ST_SCHEMA_TABLE *schema_table;
schema_table= find_schema_table(thd, &ptr->table_name); LEX_CSTRING alias_str= alias ? *alias : table->table;
ptr->schema_table_name= ptr->table_name; DBUG_ASSERT(alias_str.str);
ptr->schema_table= schema_table; if (!alias) /* Alias is case sensitive */
{
if (unlikely(table->sel))
{
my_message(ER_DERIVED_MUST_HAVE_ALIAS,
ER_THD(thd, ER_DERIVED_MUST_HAVE_ALIAS), MYF(0));
DBUG_RETURN(0);
} }
ptr->select_lex= this; /* alias_str points to table->table; Let's make a copy */
/* if (unlikely(!(alias_str.str= (char*) thd->memdup(alias_str.str, alias_str.length+1))))
We can't cache internal temporary tables between prepares as the DBUG_RETURN(0);
table may be deleted before next exection. }
*/
ptr->cacheable_table= !table->is_derived_table(); bool has_alias_ptr= alias != nullptr;
ptr->index_hints= index_hints_arg; void *memregion= thd->calloc(sizeof(TABLE_LIST));
ptr->option= option ? option->str : 0; TABLE_LIST *ptr= new (memregion) TABLE_LIST(thd, db, fqtn, alias_str,
has_alias_ptr, table, lock_type,
mdl_type, table_options,
info_schema, this,
index_hints_arg, option);
/* check that used name is unique. Sequences are ignored */ /* check that used name is unique. Sequences are ignored */
if (lock_type != TL_IGNORE && !ptr->sequence) if (lock_type != TL_IGNORE && !ptr->sequence)
{ {
...@@ -8406,6 +8378,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, ...@@ -8406,6 +8378,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
} }
} }
/* Store the table reference preceding the current one. */ /* Store the table reference preceding the current one. */
TABLE_LIST *UNINIT_VAR(previous_table_ref); /* The table preceding the current one. */
if (table_list.elements > 0 && likely(!ptr->sequence)) if (table_list.elements > 0 && likely(!ptr->sequence))
{ {
/* /*
......
...@@ -49,6 +49,7 @@ ...@@ -49,6 +49,7 @@
#include "wsrep_schema.h" #include "wsrep_schema.h"
#endif #endif
#include "log_event.h" // MAX_TABLE_MAP_ID #include "log_event.h" // MAX_TABLE_MAP_ID
#include "sql_class.h"
/* For MySQL 5.7 virtual fields */ /* For MySQL 5.7 virtual fields */
#define MYSQL57_GENERATED_FIELD 128 #define MYSQL57_GENERATED_FIELD 128
...@@ -5850,6 +5851,59 @@ void TABLE::reset_item_list(List<Item> *item_list, uint skip) const ...@@ -5850,6 +5851,59 @@ void TABLE::reset_item_list(List<Item> *item_list, uint skip) const
} }
} }
TABLE_LIST::TABLE_LIST(THD *thd,
LEX_CSTRING db_str,
bool fqtn,
LEX_CSTRING alias_str,
bool has_alias_ptr,
Table_ident *table_ident,
thr_lock_type lock_t,
enum_mdl_type mdl_t,
ulong table_opts,
bool info_schema,
st_select_lex *sel,
List<Index_hint> *index_hints_ptr,
LEX_STRING *option_ptr)
{
db= db_str;
is_fqtn= fqtn;
alias= alias_str;
is_alias= has_alias_ptr;
if (lower_case_table_names)
{
if (table_ident->table.length)
table_ident->table.length= my_casedn_str(files_charset_info,
(char*) table_ident->table.str);
if (db.length && db.str != any_db.str)
db.length= my_casedn_str(files_charset_info, (char*) db.str);
}
table_name= table_ident->table;
lock_type= lock_t;
mdl_type= mdl_t;
table_options= table_opts;
updating= table_options & TL_OPTION_UPDATING;
/* TODO: remove TL_OPTION_FORCE_INDEX as it looks like it's not used */
force_index= table_options & TL_OPTION_FORCE_INDEX;
ignore_leaves= table_options & TL_OPTION_IGNORE_LEAVES;
sequence= table_options & TL_OPTION_SEQUENCE;
derived= table_ident->sel;
if (!table_ident->sel && info_schema)
{
schema_table= find_schema_table(thd, &table_name);
schema_table_name= table_name;
}
select_lex= sel;
/*
We can't cache internal temporary tables between prepares as the
table may be deleted before next exection.
*/
cacheable_table= !table_ident->is_derived_table();
index_hints= index_hints_ptr;
option= option_ptr ? option_ptr->str : 0;
}
/* /*
calculate md5 of query calculate md5 of query
......
...@@ -2175,8 +2175,23 @@ struct TABLE_CHAIN ...@@ -2175,8 +2175,23 @@ struct TABLE_CHAIN
void set_end_pos(TABLE_LIST **pos) { end_pos= pos; } void set_end_pos(TABLE_LIST **pos) { end_pos= pos; }
}; };
class Table_ident;
struct TABLE_LIST struct TABLE_LIST
{ {
TABLE_LIST(THD *thd,
LEX_CSTRING db_str,
bool fqtn,
LEX_CSTRING alias_str,
bool has_alias_ptr,
Table_ident *table_ident,
thr_lock_type lock_t,
enum_mdl_type mdl_t,
ulong table_opts,
bool info_schema,
st_select_lex *sel,
List<Index_hint> *index_hints_ptr,
LEX_STRING *option_ptr);
TABLE_LIST() = default; /* Remove gcc warning */ TABLE_LIST() = default; /* Remove gcc warning */
enum prelocking_types enum prelocking_types
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment