revision-id: 6be8bd6ee05c6f18b6a6a0375f9bc2b68ef6c43e (mariadb-10.3.7-59-g6be8bd6) parent(s): 1abd877e2df9e83bc1c2f5195796f427a35bd3f1 committer: Alexey Botchkov timestamp: 2018-06-24 23:12:20 +0400 message: MDEV-15890 Strange error message if you try to FLUSH TABLES <view> after LOCK TABLES <view>. Check if the argument of the FLUSH TABLE is a VIEW and handle it accordingly. --- mysql-test/main/flush.result | 12 ++++ mysql-test/main/flush.test | 14 +++++ sql/sql_base.cc | 142 +++++++++++++++++++++++++++---------------- sql/sql_base.h | 3 +- sql/sql_reload.cc | 14 ++++- sql/sql_table.cc | 2 +- sql/sql_trigger.cc | 2 +- sql/sql_truncate.cc | 2 +- 8 files changed, 131 insertions(+), 60 deletions(-) diff --git a/mysql-test/main/flush.result b/mysql-test/main/flush.result index 5cd4fde..0532136 100644 --- a/mysql-test/main/flush.result +++ b/mysql-test/main/flush.result @@ -542,3 +542,15 @@ flush relay logs,relay logs; ERROR HY000: Incorrect usage of FLUSH and RELAY LOGS flush slave,slave; ERROR HY000: Incorrect usage of FLUSH and SLAVE +# +# MDEV-15890 Strange error message if you try to +# FLUSH TABLES <view> after LOCK TABLES <view>. +# +CREATE TABLE t1 (qty INT, price INT); +CREATE VIEW v1 AS SELECT qty, price, qty*price AS value FROM t1; +LOCK TABLES v1 READ; +FLUSH TABLES v1; +ERROR HY000: Table 't1' was locked with a READ lock and can't be updated +UNLOCK TABLES; +DROP VIEW v1; +DROP TABLE t1; diff --git a/mysql-test/main/flush.test b/mysql-test/main/flush.test index 81834b7..e14c8c3 100644 --- a/mysql-test/main/flush.test +++ b/mysql-test/main/flush.test @@ -673,3 +673,17 @@ DROP TABLE t1; flush relay logs,relay logs; --error ER_WRONG_USAGE flush slave,slave; +--echo # +--echo # MDEV-15890 Strange error message if you try to +--echo # FLUSH TABLES <view> after LOCK TABLES <view>. +--echo # + +CREATE TABLE t1 (qty INT, price INT); +CREATE VIEW v1 AS SELECT qty, price, qty*price AS value FROM t1; +LOCK TABLES v1 READ; +--error ER_TABLE_NOT_LOCKED_FOR_WRITE +FLUSH TABLES v1; +UNLOCK TABLES; +DROP VIEW v1; +DROP TABLE t1; + diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 80939cb..8a6a509 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -414,9 +414,10 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, for (TABLE_LIST *table_list= tables_to_reopen; table_list; table_list= table_list->next_global) { + int err; /* A check that the table was locked for write is done by the caller. */ TABLE *table= find_table_for_mdl_upgrade(thd, table_list->db.str, - table_list->table_name.str, TRUE); + table_list->table_name.str, &err); /* May return NULL if this table has already been closed via an alias. */ if (! table) @@ -1493,6 +1494,67 @@ static int set_partitions_as_used(TABLE_LIST *tl, TABLE *t) /** + Check if the given table is actually a VIEW that was LOCK-ed + + @param thd Thread context. + @param t Table to check. + + @retval TRUE The 't'-table is a locked view + needed to remedy problem before retrying again. + @retval FALSE 't' was not locked, not a VIEW or an error happened. +*/ +bool is_locked_view(THD *thd, TABLE_LIST *t) +{ + DBUG_ENTER("check_locked_view"); + /* + Is this table a view and not a base table? + (it is work around to allow to open view with locked tables, + real fix will be made after definition cache will be made) + + Since opening of view which was not explicitly locked by LOCK + TABLES breaks metadata locking protocol (potentially can lead + to deadlocks) it should be disallowed. + */ + if (thd->mdl_context.is_lock_owner(MDL_key::TABLE, + t->db.str, + t->table_name.str, + MDL_SHARED)) + { + char path[FN_REFLEN + 1]; + build_table_filename(path, sizeof(path) - 1, + t->db.str, t->table_name.str, reg_ext, 0); + /* + Note that we can't be 100% sure that it is a view since it's + possible that we either simply have not found unused TABLE + instance in THD::open_tables list or were unable to open table + during prelocking process (in this case in theory we still + should hold shared metadata lock on it). + */ + if (dd_frm_is_view(thd, path)) + { + /* + If parent_l of the table_list is non null then a merge table + has this view as child table, which is not supported. + */ + if (t->parent_l) + { + my_error(ER_WRONG_MRG_TABLE, MYF(0)); + DBUG_RETURN(FALSE); + } + + if (!tdc_open_view(thd, t, CHECK_METADATA_VERSION)) + { + DBUG_ASSERT(t->view != 0); + DBUG_RETURN(TRUE); // VIEW + } + } + } + + DBUG_RETURN(FALSE); +} + + +/** Open a base table. @param thd Thread context. @@ -1640,49 +1702,10 @@ bool open_table(THD *thd, TABLE_LIST *table_list, Open_table_context *ot_ctx) #endif goto reset; } - /* - Is this table a view and not a base table? - (it is work around to allow to open view with locked tables, - real fix will be made after definition cache will be made) - Since opening of view which was not explicitly locked by LOCK - TABLES breaks metadata locking protocol (potentially can lead - to deadlocks) it should be disallowed. - */ - if (thd->mdl_context.is_lock_owner(MDL_key::TABLE, - table_list->db.str, - table_list->table_name.str, - MDL_SHARED)) - { - char path[FN_REFLEN + 1]; - build_table_filename(path, sizeof(path) - 1, - table_list->db.str, table_list->table_name.str, reg_ext, 0); - /* - Note that we can't be 100% sure that it is a view since it's - possible that we either simply have not found unused TABLE - instance in THD::open_tables list or were unable to open table - during prelocking process (in this case in theory we still - should hold shared metadata lock on it). - */ - if (dd_frm_is_view(thd, path)) - { - /* - If parent_l of the table_list is non null then a merge table - has this view as child table, which is not supported. - */ - if (table_list->parent_l) - { - my_error(ER_WRONG_MRG_TABLE, MYF(0)); - DBUG_RETURN(true); - } + if (is_locked_view(thd, table_list)) + DBUG_RETURN(FALSE); // VIEW - if (!tdc_open_view(thd, table_list, CHECK_METADATA_VERSION)) - { - DBUG_ASSERT(table_list->view != 0); - DBUG_RETURN(FALSE); // VIEW - } - } - } /* No table in the locked tables list. In case of explicit LOCK TABLES this can happen if a user did not include the table into the list. @@ -2052,8 +2075,9 @@ TABLE *find_locked_table(TABLE *list, const char *db, const char *table_name) @param thd Thread context @param db Database name. @param table_name Name of table. - @param no_error Don't emit error if no suitable TABLE - instance were found. + @param p_error In the case of an error (when the function returns NULL) + the error number is stored there. + If the p_error is NULL, function launches the error itself. @note This function checks if the connection holds a global IX metadata lock. If no such lock is found, it is not safe to @@ -2066,15 +2090,15 @@ TABLE *find_locked_table(TABLE *list, const char *db, const char *table_name) */ TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db, - const char *table_name, bool no_error) + const char *table_name, int *p_error) { TABLE *tab= find_locked_table(thd->open_tables, db, table_name); + int error; if (unlikely(!tab)) { - if (!no_error) - my_error(ER_TABLE_NOT_LOCKED, MYF(0), table_name); - return NULL; + error= ER_TABLE_NOT_LOCKED; + goto err_exit; } /* @@ -2086,9 +2110,8 @@ TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db, if (unlikely(!thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, "", "", MDL_INTENTION_EXCLUSIVE))) { - if (!no_error) - my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name); - return NULL; + error= ER_TABLE_NOT_LOCKED_FOR_WRITE; + goto err_exit; } while (tab->mdl_ticket != NULL && @@ -2096,10 +2119,21 @@ TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db, (tab= find_locked_table(tab->next, db, table_name))) continue; - if (unlikely(!tab && !no_error)) - my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name); + if (unlikely(!tab)) + { + error= ER_TABLE_NOT_LOCKED_FOR_WRITE; + goto err_exit; + } return tab; + +err_exit: + if (p_error) + *p_error= error; + else + my_error(error, MYF(0), table_name); + + return NULL; } @@ -3905,7 +3939,7 @@ open_tables_check_upgradable_mdl(THD *thd, TABLE_LIST *tables_start, Note that find_table_for_mdl_upgrade() will report an error if no suitable ticket is found. */ - if (!find_table_for_mdl_upgrade(thd, table->db.str, table->table_name.str, false)) + if (!find_table_for_mdl_upgrade(thd, table->db.str, table->table_name.str, NULL)) return TRUE; } diff --git a/sql/sql_base.h b/sql/sql_base.h index 01892c0..d4d9428 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -125,6 +125,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update, MYSQL_OPEN_GET_NEW_TABLE |\ MYSQL_OPEN_HAS_MDL_LOCK) +bool is_locked_view(THD *thd, TABLE_LIST *t); bool open_table(THD *thd, TABLE_LIST *table_list, Open_table_context *ot_ctx); bool get_key_map_from_key_list(key_map *map, TABLE *table, @@ -299,7 +300,7 @@ bool tdc_open_view(THD *thd, TABLE_LIST *table_list, uint flags); TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db, const char *table_name, - bool no_error); + int *p_error); int dynamic_column_error_message(enum_dyncol_func_result rc); diff --git a/sql/sql_reload.cc b/sql/sql_reload.cc index 9bcb9a3..c9d854e 100644 --- a/sql/sql_reload.cc +++ b/sql/sql_reload.cc @@ -30,6 +30,7 @@ #include "sql_show.h" #include "debug_sync.h" #include "des_key_file.h" +#include "sql_view.h" #include "transaction.h" static void disable_checkpoints(THD *thd); @@ -289,9 +290,18 @@ bool reload_acl_and_cache(THD *thd, unsigned long long options, */ if (tables) { + int err; for (TABLE_LIST *t= tables; t; t= t->next_local) - if (!find_table_for_mdl_upgrade(thd, t->db.str, t->table_name.str, false)) - return 1; + if (!find_table_for_mdl_upgrade(thd, t->db.str, t->table_name.str, &err)) + { + if (is_locked_view(thd, t)) + t->next_local= t->next_global; + else + { + my_error(err, MYF(0), t->table_name); + return 1; + } + } } else { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index ac65fbb..f39ddce 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2113,7 +2113,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, bool if_exists, in its elements. */ table->table= find_table_for_mdl_upgrade(thd, table->db.str, - table->table_name.str, false); + table->table_name.str, NULL); if (!table->table) DBUG_RETURN(true); table->mdl_request.ticket= table->table->mdl_ticket; diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index df3a2dc..dfc8cfd 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -543,7 +543,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) /* Under LOCK TABLES we must only accept write locked tables. */ if (!(tables->table= find_table_for_mdl_upgrade(thd, tables->db.str, tables->table_name.str, - FALSE))) + NULL))) goto end; } else diff --git a/sql/sql_truncate.cc b/sql/sql_truncate.cc index cf91d6d..09d6e98 100644 --- a/sql/sql_truncate.cc +++ b/sql/sql_truncate.cc @@ -299,7 +299,7 @@ bool Sql_cmd_truncate_table::lock_table(THD *thd, TABLE_LIST *table_ref, if (thd->locked_tables_mode) { if (!(table= find_table_for_mdl_upgrade(thd, table_ref->db.str, - table_ref->table_name.str, FALSE))) + table_ref->table_name.str, NULL))) DBUG_RETURN(TRUE); *hton_can_recreate= ha_check_storage_engine_flag(table->file->ht,