Skip to content

Commit e359ebc

Browse files
gopshankdahlerlend
authored andcommitted
WL#6599 - New Data Dictionary and I_S integration.
Post-push patch fixing POINT_SELECT performance regression fix. The patch does following improvement in open_tables() call flow, 1 We were invoking dd::Dictionary::is_system_view_name() several times for a table. E.g., SELECT_LEX::add_table_to_list() would already know that if a TABLE_LIST is a system view. We were not setting TABLE_LIST->is_system_view here. This patch sets this in SELECT_LEX::add_table_to_list() and avoids calls to dd::Dictionary::is_system_view_name() function call which goes throw all the system view names within the open_tables() call flow. 2 We also avoid call to dd::Dictionary::is_dd_table_name(). Basically there are two possibilities of open_tables() call seeing a DD tables. a) DD table being opened as part of DD operations invoking dd::Open_dictionary_tables_ctx::open_tables(). b) DD table being opened as part of I_S system view execution. During open_table(), we need to know if a TABLE_LIST belongs to a DD table for several checks. Currently we do that by invoking dd::Dictionary::is_dd_table_name() which is a looking in a list. We can avoid that as described below. When open_table() is opening a DD table in case of a), the table_list used there is nothing but dd::Raw_table::m_table_list. And we are sure that this belongs to only DD tables. So, this patch adds a member TABLE_LIST->is_dd_table, which is set only by dd::Raw_table. So the open_table() call now uses it. For b), we know that TABLE_LIST->is_system_view is marked for all the I_S system views. And TABLE_LIST->referencing_view->is_system_view would tell us that the TABLE_LIST is refering to a DD table. So, this avoids calls to dd::Dictionary::is_system_view_name(). 3 The function TABLE_SHARE::get_table_ref_version() is invoking both dd::Dictionary::is_dd_table_name() and dd::Dictionary::is_system_view_name(). This is a overhead. This patch adds a TABLE_SHARE->table_category called TABLE_CATEGORY_DICTIONARY. This helps us avoid call to is_dd_table_name(). And use TABLE_SHARE->view_object->type() == dd::enum_table_type::SYSTEM_VIEW to check if that is a system view. 4 The patch does following change, that is not necessarily to improve the permformance. Basically the revno 56eaef86 sets MYSQL_OPEN_IGNORE_FLUSH flag while opening DD tables in Open_dictionary_tables_ctx::open_tables(). And later the revno 11eeb00a removes this flag, expecting open_table() to set it for DD tables. Conceptually it looks correct to set this flag for all DD table at Open_dictionary_tables_ctx::open_tables() so this patch retain setting of this flag as done by revno 56eaef86. These revno are from mysql-trunk-wl6599-1 branch.
1 parent 15c2a56 commit e359ebc

File tree

11 files changed

+76
-48
lines changed

11 files changed

+76
-48
lines changed

sql/dd/dictionary.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ class Dictionary
114114
@returns true - If given table name is a system view.
115115
@returns false - If table name is not a system view.
116116
*/
117-
virtual bool is_system_view_name(const std::string &schema_name,
118-
const std::string &table_name) const = 0;
117+
virtual bool is_system_view_name(const char *schema_name,
118+
const char *table_name) const = 0;
119119

120120
public:
121121
// Destructor to cleanup data dictionary instance upon server shutdown.

sql/dd/impl/dictionary_impl.cc

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -185,23 +185,29 @@ bool Dictionary_impl::is_dd_table_access_allowed(
185185

186186
///////////////////////////////////////////////////////////////////////////
187187

188-
bool Dictionary_impl::is_system_view_name(const std::string &schema_name,
189-
const std::string &table_name) const
188+
bool Dictionary_impl::is_system_view_name(const char *schema_name,
189+
const char *table_name) const
190190
{
191-
// The System_views registry stores the view name in lowercase.
192-
// So convert the input to lowercase before search.
193-
char sch_name_buf[NAME_LEN + 1];
194-
my_stpcpy(sch_name_buf, schema_name.c_str());
195-
my_casedn_str(&my_charset_utf8_tolower_ci, sch_name_buf);
196-
197-
if (strcmp(sch_name_buf,INFORMATION_SCHEMA_NAME.str) != 0)
191+
/*
192+
TODO One possible improvement here could be to try and use the variant
193+
of is_infoschema_db() that takes length as a parameter. Then, if the
194+
schema name length is different, this can quickly be used to conclude
195+
that this is indeed not a system view, without having to do a strcmp at
196+
all.
197+
*/
198+
if (schema_name == nullptr ||
199+
table_name == nullptr ||
200+
is_infoschema_db(schema_name) == false)
198201
return false;
199202

203+
// The System_views registry stores the view name in lowercase.
204+
// So convert the input to lowercase before search.
200205
char tab_name_buf[NAME_LEN + 1];
201-
my_stpcpy(tab_name_buf, table_name.c_str());
202-
my_caseup_str(&my_charset_utf8_tolower_ci, tab_name_buf);
206+
my_stpcpy(tab_name_buf, table_name);
207+
my_caseup_str(system_charset_info, tab_name_buf);
203208

204-
return (System_views::instance()->find(sch_name_buf, tab_name_buf) != NULL);
209+
return (System_views::instance()->find(INFORMATION_SCHEMA_NAME.str,
210+
tab_name_buf) != NULL);
205211
}
206212

207213
///////////////////////////////////////////////////////////////////////////

sql/dd/impl/dictionary_impl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ class Dictionary_impl : public Dictionary
9393
size_t schema_length,
9494
const char *table_name) const;
9595

96-
virtual bool is_system_view_name(const std::string &schema_name,
97-
const std::string &table_name) const;
96+
virtual bool is_system_view_name(const char *schema_name,
97+
const char *table_name) const;
9898

9999
public:
100100
static Object_id default_catalog_id()

sql/dd/impl/raw/raw_table.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ Raw_table::Raw_table(thr_lock_type lock_type,
3232
name.length(),
3333
name.c_str(),
3434
lock_type);
35+
m_table_list.is_dd_ctx_table= true;
3536
}
3637

3738
///////////////////////////////////////////////////////////////////////////

sql/dd/impl/transaction_impl.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ bool Open_dictionary_tables_ctx::open_tables()
9292

9393
const uint flags= (MYSQL_LOCK_IGNORE_TIMEOUT |
9494
MYSQL_OPEN_IGNORE_KILLED |
95+
MYSQL_OPEN_IGNORE_FLUSH |
9596
m_ignore_global_read_lock ?
9697
MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK : 0);
9798
uint counter;

sql/sp_head.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3518,6 +3518,8 @@ void sp_head::add_used_tables_to_table_list(THD *thd,
35183518
stab->table_name_length + 1,
35193519
stab->lock_type, mdl_lock_type);
35203520

3521+
table->is_system_view=
3522+
dd::get_dictionary()->is_system_view_name(table->db, table->table_name);
35213523
table->cacheable_table= 1;
35223524
table->prelocking_placeholder= 1;
35233525
table->belong_to_view= belong_to_view;

sql/sql_base.cc

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,11 +1374,26 @@ static inline bool in_LTM(THD *thd)
13741374
thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES);
13751375
}
13761376

1377-
// Check if the given db.table belongs to a IS view or a DD table.
1378-
bool belongs_to_system_view(const char *db, const char *table_name)
1377+
1378+
/**
1379+
Check if the given TABLE_LIST belongs to a a DD table.
1380+
1381+
The function checks whether the table is a DD table being used in the
1382+
context of a DD transaction, or whether it is referred by a system view.
1383+
Then, it implies that if either of these two conditions hold, then this
1384+
is a DD table. If in case this is a DD table being used in some other
1385+
situation, then this function does not return 'true'. We do not know if
1386+
there is such a situation right now.
1387+
1388+
@param tl TABLE_LIST point to the table.
1389+
1390+
@retval true If table belongs to a DD table.
1391+
@retval false If table does not.
1392+
*/
1393+
static bool belongs_to_dd_table(const TABLE_LIST *tl)
13791394
{
1380-
return (dd::get_dictionary()->is_dd_table_name(db, table_name) ||
1381-
dd::get_dictionary()->is_system_view_name(db, table_name));
1395+
return (tl->is_dd_ctx_table ||
1396+
(tl->referencing_view && tl->referencing_view->is_system_view));
13821397
}
13831398

13841399

@@ -1526,8 +1541,7 @@ void close_thread_tables(THD *thd)
15261541
*/
15271542
TABLE_LIST *tbl_list= table->pos_in_table_list;
15281543
if (!thd->in_sub_stmt &&
1529-
(dd::get_dictionary()->is_dd_table_name(
1530-
tbl_list->db, tbl_list->table_name) ||
1544+
(belongs_to_dd_table(tbl_list) ||
15311545
belongs_to_p_s(table->pos_in_table_list)))
15321546
{
15331547
if (!table->s->tmp_table)
@@ -2931,7 +2945,8 @@ bool open_table(THD *thd, TABLE_LIST *table_list, Open_table_context *ot_ctx)
29312945
FLUSH TABLES is ignored for DD, I_S and P_S tables/views.
29322946
Hence setting MYSQL_OPEN_IGNORE_FLUSH flag.
29332947
*/
2934-
if (belongs_to_system_view(table_list->db, table_list->table_name) ||
2948+
if (table_list->is_system_view ||
2949+
belongs_to_dd_table(table_list) ||
29352950
belongs_to_p_s(table_list))
29362951
flags|= MYSQL_OPEN_IGNORE_FLUSH;
29372952

@@ -2964,7 +2979,8 @@ bool open_table(THD *thd, TABLE_LIST *table_list, Open_table_context *ot_ctx)
29642979
if (thd->locked_tables_mode &&
29652980
!(flags & MYSQL_OPEN_GET_NEW_TABLE) &&
29662981
!(in_LTM(thd) &&
2967-
(belongs_to_system_view(table_list->db, table_list->table_name) ||
2982+
(table_list->is_system_view ||
2983+
belongs_to_dd_table(table_list) ||
29682984
belongs_to_p_s(table_list))))
29692985
{ // Using table locks
29702986
TABLE *best_table= 0;
@@ -5450,8 +5466,7 @@ get_and_lock_tablespace_names(THD *thd,
54505466
table->open_type != OT_TEMPORARY_ONLY &&
54515467
!(table->open_type == OT_TEMPORARY_OR_BASE &&
54525468
is_temporary_table(table)) &&
5453-
!dd::get_dictionary()->is_system_view_name(table->db,
5454-
table->table_name))
5469+
!table->is_system_view)
54555470
{
54565471
// We have basically three situations here:
54575472
//
@@ -6037,8 +6052,7 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
60376052
by SE. Here, we request SE to use read lock for these implicitly opened
60386053
DD tables using ha_external_lock().
60396054
*/
6040-
if (tbl && in_LTM(thd) &&
6041-
dd::get_dictionary()->is_dd_table_name(tables->db, tables->table_name))
6055+
if (tbl && in_LTM(thd) && belongs_to_dd_table(tables))
60426056
{
60436057
DBUG_ASSERT(tbl->file->get_lock_type() == F_UNLCK);
60446058
tbl->file->init_table_handle_for_HANDLER();

sql/sql_base.h

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -349,19 +349,6 @@ TABLE_LIST *find_table_in_global_list(TABLE_LIST *table,
349349
const char *db_name,
350350
const char *table_name);
351351

352-
/**
353-
Check if the given db.table belongs to a IS view or a DD table.
354-
355-
@param db Database name.
356-
@param table_name Table name.
357-
358-
@retval true If db.table belongs to a IS view or a DD table.
359-
@retval false If db.table does not belongs to a IS view or a
360-
DD table.
361-
*/
362-
363-
bool belongs_to_system_view(const char *db, const char *table_name);
364-
365352
/**
366353
An abstract class for a strategy specifying how the prelocking
367354
algorithm should extend the prelocking set while processing

sql/sql_parse.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5571,23 +5571,23 @@ TABLE_LIST *SELECT_LEX::add_table_to_list(THD *thd,
55715571
const_cast<char*>(ptr->db),
55725572
const_cast<char*>(ptr->table_name));
55735573

5574-
bool is_system_view=
5574+
ptr->is_system_view=
55755575
dd::get_dictionary()->is_system_view_name(ptr->db, ptr->table_name);
55765576

55775577
ST_SCHEMA_TABLE *schema_table;
55785578
if (ptr->updating &&
55795579
/* Special cases which are processed by commands itself */
55805580
lex->sql_command != SQLCOM_CHECK &&
55815581
lex->sql_command != SQLCOM_CHECKSUM &&
5582-
!(lex->sql_command == SQLCOM_CREATE_VIEW && is_system_view))
5582+
!(lex->sql_command == SQLCOM_CREATE_VIEW && ptr->is_system_view))
55835583
{
55845584
my_error(ER_DBACCESS_DENIED_ERROR, MYF(0),
55855585
thd->security_context()->priv_user().str,
55865586
thd->security_context()->priv_host().str,
55875587
INFORMATION_SCHEMA_NAME.str);
55885588
DBUG_RETURN(0);
55895589
}
5590-
if (is_system_view)
5590+
if (ptr->is_system_view)
55915591
{
55925592
/**
55935593
Pick the right IS system view definition based on session

sql/table.cc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
4747
#include "table_trigger_dispatcher.h" // Table_trigger_dispatcher
4848
#include "template_utils.h" // down_cast
4949

50+
#include "dd/dd.h" // dd::get_dictionary
51+
#include "dd/dictionary.h" // dd::Dictionary
5052
#include "dd/types/table.h" // dd::Table
5153
#include "dd/types/view.h" // dd::View
5254

@@ -328,6 +330,9 @@ TABLE_CATEGORY get_table_category(const LEX_STRING &db,
328330
name.str) == 0))
329331
return TABLE_CATEGORY_GTID;
330332

333+
if (dd::get_dictionary()->is_dd_table_name(MYSQL_SCHEMA_NAME.str,
334+
name.str))
335+
return TABLE_CATEGORY_DICTIONARY;
331336
}
332337

333338
return TABLE_CATEGORY_USER;
@@ -2431,9 +2436,10 @@ bool TABLE_SHARE::wait_for_old_version(THD *thd, struct timespec *abstime,
24312436

24322437
ulonglong TABLE_SHARE::get_table_ref_version() const
24332438
{
2434-
if (belongs_to_system_view(static_cast<const char *>(db.str),
2435-
static_cast<const char *>(table_name.str)) ||
2436-
(tmp_table == SYSTEM_TMP_TABLE))
2439+
if (table_category == TABLE_CATEGORY_DICTIONARY ||
2440+
tmp_table == SYSTEM_TMP_TABLE ||
2441+
(is_view && view_object &&
2442+
view_object->type() == dd::enum_table_type::SYSTEM_VIEW))
24372443
return 0;
24382444

24392445
return table_map_id.id();

sql/table.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,15 @@ enum enum_table_category
482482
a GLOBAL READ LOCK or a GLOBAL READ_ONLY in effect.
483483
Gtid table is cached in the table cache.
484484
*/
485-
TABLE_CATEGORY_GTID=8
485+
TABLE_CATEGORY_GTID=8,
486+
487+
/**
488+
A data dictionary table.
489+
Table's with this category will skip checking the
490+
TABLE_SHARE versions because these table structures
491+
are fixed upon server bootstrap.
492+
*/
493+
TABLE_CATEGORY_DICTIONARY=9
486494
};
487495
typedef enum enum_table_category TABLE_CATEGORY;
488496

@@ -2601,6 +2609,9 @@ struct TABLE_LIST
26012609
// True, If this is a system view
26022610
bool is_system_view;
26032611

2612+
// True, If this is a dictionary table.
2613+
bool is_dd_ctx_table;
2614+
26042615
/* End of view definition context. */
26052616

26062617
/* List of possible keys. Valid only for materialized derived tables/views. */

0 commit comments

Comments
 (0)