commit fa31a09e5ae489c00e340d6530c9ba995887d6f0 Author: Dmitry Lenev Date: Wed Aug 16 14:13:15 2023 +0200 Bug#112011 "RESOURCE GROUP hint fails when used within a prepared statement". RESOURCE_GROUP hints specified in the text of prepared statement didn't apply during its execution. The problem occurred because information about RESOURCE_GROUP hint (the resource group to be used for statement execution) was associated with connection context (a.k.a. THD) and not with prepared statement context. Moreover, to avoid situation when hint specified for a statement affected execution of further statements in connection, it was reset after execution of the statement. Which is not re-execution safe technique. This patch solves the problem by associating information about this hint with LEX object which is part of prepared statement context. The problem with a hint specified for a statement affecting execution of further statements in the connection is solved automatically in this case, since LEX is re-inited before parsing each statement. diff --git a/mysql-test/r/resource_group_bugs.result b/mysql-test/r/resource_group_bugs.result index 4d327e50b81..c33c09c6536 100644 --- a/mysql-test/r/resource_group_bugs.result +++ b/mysql-test/r/resource_group_bugs.result @@ -21,3 +21,60 @@ CREATE RESOURCE GROUP CAFE TYPE=USER VCPU=1-3 THREAD_PRIORITY=5; ERROR HY000: Resource Group 'CAFE' exists DROP RESOURCE GROUP CaFé; SET NAMES default; +# +# Bug#112011 "RESOURCE GROUP hint fails when used within a prepared statement". +# +CREATE RESOURCE GROUP r1 TYPE=USER VCPU=0,1; +CREATE RESOURCE GROUP r2 TYPE=USER VCPU=2,3; +# The below query sees itself as executed in 'r1' resource group +# in the P_S.THREADS table. +SELECT /*+ RESOURCE_GROUP(r1) */ processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id(); +processlist_info resource_group +SELECT /*+ RESOURCE_GROUP(r1) */ processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id() r1 +# Make prepared statement from the same query. +PREPARE stmt1 FROM 'SELECT /*+ RESOURCE_GROUP(r1) */ processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id()'; +# Execution of prepared statement should use 'r1' resource group +# as well (it didn't before the fix). +EXECUTE stmt1; +processlist_info resource_group +EXECUTE stmt1 r1 +# Check that hint applies in case of re-execution. +EXECUTE stmt1; +processlist_info resource_group +EXECUTE stmt1 r1 +# Check that further statements in the connection are not affected. +SELECT processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id(); +processlist_info resource_group +SELECT processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id() USR_default +# Now let us check that we can have several prepared statements using +# different resource groups within the same connection. +PREPARE stmt2 FROM 'SELECT /*+ RESOURCE_GROUP(r2) */ processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id()'; +PREPARE stmt3 FROM 'SELECT processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id()'; +# The first statement should still use 'r1', +EXECUTE stmt1; +processlist_info resource_group +EXECUTE stmt1 r1 +# The second statement should use 'r2', +EXECUTE stmt2; +processlist_info resource_group +EXECUTE stmt2 r2 +# The third one should used default group. +EXECUTE stmt3; +processlist_info resource_group +EXECUTE stmt3 USR_default +# Ditto for case of re-execution. +EXECUTE stmt1; +processlist_info resource_group +EXECUTE stmt1 r1 +EXECUTE stmt2; +processlist_info resource_group +EXECUTE stmt2 r2 +EXECUTE stmt3; +processlist_info resource_group +EXECUTE stmt3 USR_default +# Clean-up. +DEALLOCATE PREPARE stmt1; +DEALLOCATE PREPARE stmt2; +DEALLOCATE PREPARE stmt3; +DROP RESOURCE GROUP r1; +DROP RESOURCE GROUP r2; diff --git a/mysql-test/t/resource_group_bugs.test b/mysql-test/t/resource_group_bugs.test index 860976c718e..c20bde34733 100644 --- a/mysql-test/t/resource_group_bugs.test +++ b/mysql-test/t/resource_group_bugs.test @@ -28,3 +28,48 @@ CREATE RESOURCE GROUP CAFE TYPE=USER VCPU=1-3 THREAD_PRIORITY=5; DROP RESOURCE GROUP CaFé; SET NAMES default; + + +--echo # +--echo # Bug#112011 "RESOURCE GROUP hint fails when used within a prepared statement". +--echo # + +CREATE RESOURCE GROUP r1 TYPE=USER VCPU=0,1; +CREATE RESOURCE GROUP r2 TYPE=USER VCPU=2,3; + +--echo # The below query sees itself as executed in 'r1' resource group +--echo # in the P_S.THREADS table. +SELECT /*+ RESOURCE_GROUP(r1) */ processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id(); + +--echo # Make prepared statement from the same query. +PREPARE stmt1 FROM 'SELECT /*+ RESOURCE_GROUP(r1) */ processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id()'; +--echo # Execution of prepared statement should use 'r1' resource group +--echo # as well (it didn't before the fix). +EXECUTE stmt1; +--echo # Check that hint applies in case of re-execution. +EXECUTE stmt1; + +--echo # Check that further statements in the connection are not affected. +SELECT processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id(); + +--echo # Now let us check that we can have several prepared statements using +--echo # different resource groups within the same connection. +PREPARE stmt2 FROM 'SELECT /*+ RESOURCE_GROUP(r2) */ processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id()'; +PREPARE stmt3 FROM 'SELECT processlist_info, resource_group FROM performance_schema.threads WHERE processlist_id = connection_id()'; +--echo # The first statement should still use 'r1', +EXECUTE stmt1; +--echo # The second statement should use 'r2', +EXECUTE stmt2; +--echo # The third one should used default group. +EXECUTE stmt3; +--echo # Ditto for case of re-execution. +EXECUTE stmt1; +EXECUTE stmt2; +EXECUTE stmt3; + +--echo # Clean-up. +DEALLOCATE PREPARE stmt1; +DEALLOCATE PREPARE stmt2; +DEALLOCATE PREPARE stmt3; +DROP RESOURCE GROUP r1; +DROP RESOURCE GROUP r2; diff --git a/sql/parse_tree_hints.cc b/sql/parse_tree_hints.cc index f06d1c188b5..cb3b0130aa4 100644 --- a/sql/parse_tree_hints.cc +++ b/sql/parse_tree_hints.cc @@ -577,9 +577,12 @@ bool PT_hint_resource_group::contextualize(Parse_context *pc) { return false; } - memcpy(pc->thd->resource_group_ctx()->m_switch_resource_group_str, - m_resource_group_name.str, m_resource_group_name.length); - pc->thd->resource_group_ctx() - ->m_switch_resource_group_str[m_resource_group_name.length] = '\0'; + /* + In case of duplicate hints the last one takes precedence. + Extra safety - treat empty group name as no hint. + */ + pc->thd->lex->switch_resource_group = + m_resource_group_name.length ? m_resource_group_name.str : nullptr; + return false; } diff --git a/sql/resourcegroups/resource_group_basic_types.h b/sql/resourcegroups/resource_group_basic_types.h index d0fd719900d..f5cc8d33a90 100644 --- a/sql/resourcegroups/resource_group_basic_types.h +++ b/sql/resourcegroups/resource_group_basic_types.h @@ -52,7 +52,6 @@ struct Range { class Resource_group; struct Resource_group_ctx { Resource_group *m_cur_resource_group; - char m_switch_resource_group_str[NAME_CHAR_LEN + 1]; int m_warn; }; } // namespace resourcegroups diff --git a/sql/resourcegroups/resource_group_mgr.cc b/sql/resourcegroups/resource_group_mgr.cc index e309ad32dd1..d82f63ec33a 100644 --- a/sql/resourcegroups/resource_group_mgr.cc +++ b/sql/resourcegroups/resource_group_mgr.cc @@ -536,9 +536,9 @@ bool Resource_group_mgr::switch_resource_group_if_needed( resourcegroups::Resource_group **dest_res_grp, MDL_ticket **ticket, MDL_ticket **cur_ticket) { bool switched = false; - auto res_grp_name = thd->resource_group_ctx()->m_switch_resource_group_str; - if (!opt_initialize && res_grp_name[0] != '\0') { + if (!opt_initialize && thd->lex->switch_resource_group != nullptr) { + auto res_grp_name = thd->lex->switch_resource_group; resourcegroups::Resource_group_mgr *mgr_instance = resourcegroups::Resource_group_mgr::instance(); @@ -546,7 +546,6 @@ bool Resource_group_mgr::switch_resource_group_if_needed( thd, res_grp_name, MDL_EXPLICIT, ticket, false)) { LogErr(WARNING_LEVEL, ER_FAILED_TO_ACQUIRE_LOCK_ON_RESOURCE_GROUP, res_grp_name); - res_grp_name[0] = '\0'; return false; } @@ -589,7 +588,6 @@ bool Resource_group_mgr::switch_resource_group_if_needed( LogErr(WARNING_LEVEL, ER_FAILED_TO_ACQUIRE_LOCK_ON_RESOURCE_GROUP, src_res_grp_str); mysql_mutex_unlock(&thd->LOCK_thd_data); - res_grp_name[0] = '\0'; return false; } assert(*dest_res_grp != nullptr); diff --git a/sql/sql_class.cc b/sql/sql_class.cc index c623145df75..361c21e7041 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -764,7 +764,6 @@ THD::THD(bool enable_plugins) peer_port = 0; // For SHOW PROCESSLIST get_transaction()->m_flags.enabled = true; m_resource_group_ctx.m_cur_resource_group = nullptr; - m_resource_group_ctx.m_switch_resource_group_str[0] = '\0'; m_resource_group_ctx.m_warn = 0; m_safe_to_display.store(false); diff --git a/sql/sql_class.h b/sql/sql_class.h index 2b72c9300b3..3f01b0daaab 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1014,8 +1014,7 @@ class THD : public MDL_context_owner, /** Resource group context indicating the current resource group - and the name of the resource group to switch to during execution - of a query. + and if there are any warnings related to switching resource group. */ resourcegroups::Resource_group_ctx m_resource_group_ctx; diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 686e0f5787f..3f309c75c60 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -479,6 +479,7 @@ void LEX::reset() { mark_broken(false); reset_exec_started(); max_execution_time = 0; + switch_resource_group = nullptr; reparse_common_table_expr_at = 0; reparse_derived_table_condition = false; opt_hints_global = nullptr; diff --git a/sql/sql_lex.h b/sql/sql_lex.h index d225f25d868..a2ba1c45505 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -4155,6 +4155,9 @@ struct LEX : public Query_tables_list { // Maximum execution time for a statement. ulong max_execution_time; + /// Value of RESOURCE_GROUP hint for a statement (nullptr if no hint). + const char *switch_resource_group; + /* To flag the current statement as dependent for binary logging on explicit_defaults_for_timestamp diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 64f26eb21a9..e4fb35111d9 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3027,7 +3027,6 @@ int mysql_execute_command(THD *thd, bool first_level) { } if (thd->resource_group_ctx()->m_warn != 0) { - auto res_grp_name = thd->resource_group_ctx()->m_switch_resource_group_str; switch (thd->resource_group_ctx()->m_warn) { case WARN_RESOURCE_GROUP_UNSUPPORTED: { auto res_grp_mgr = resourcegroups::Resource_group_mgr::instance(); @@ -3053,18 +3052,23 @@ int mysql_execute_command(THD *thd, bool first_level) { #ifdef HAVE_PSI_THREAD_INTERFACE pfs_thread_id = PSI_THREAD_CALL(get_current_thread_internal_id)(); #endif // HAVE_PSI_THREAD_INTERFACE + // Resource group name is always specified for this type of warning. + assert(thd->lex->switch_resource_group != nullptr); push_warning_printf(thd, Sql_condition::SL_WARNING, ER_RESOURCE_GROUP_BIND_FAILED, ER_THD(thd, ER_RESOURCE_GROUP_BIND_FAILED), - res_grp_name, pfs_thread_id, + thd->lex->switch_resource_group, pfs_thread_id, "System resource group can't be bound" " with a session thread"); break; } case WARN_RESOURCE_GROUP_NOT_EXISTS: - push_warning_printf( - thd, Sql_condition::SL_WARNING, ER_RESOURCE_GROUP_NOT_EXISTS, - ER_THD(thd, ER_RESOURCE_GROUP_NOT_EXISTS), res_grp_name); + // Resource group name is always specified for this type of warning. + assert(thd->lex->switch_resource_group != nullptr); + push_warning_printf(thd, Sql_condition::SL_WARNING, + ER_RESOURCE_GROUP_NOT_EXISTS, + ER_THD(thd, ER_RESOURCE_GROUP_NOT_EXISTS), + thd->lex->switch_resource_group); break; case WARN_RESOURCE_GROUP_ACCESS_DENIED: push_warning_printf(thd, Sql_condition::SL_WARNING, @@ -3074,7 +3078,6 @@ int mysql_execute_command(THD *thd, bool first_level) { "RESOURCE_GROUP_USER"); } thd->resource_group_ctx()->m_warn = 0; - res_grp_name[0] = '\0'; } if (unlikely(thd->get_protocol()->has_client_capability(CLIENT_NO_SCHEMA))) { @@ -5370,7 +5373,6 @@ void dispatch_sql_command(THD *thd, Parser_state *parser_state) { if (switched) mgr_ptr->restore_original_resource_group(thd, src_res_grp, dest_res_grp); - thd->resource_group_ctx()->m_switch_resource_group_str[0] = '\0'; if (ticket != nullptr) mgr_ptr->release_shared_mdl_for_resource_group(thd, ticket); if (cur_ticket != nullptr) diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index d2b6353a038..45c73f3dc81 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -3500,7 +3500,6 @@ bool Prepared_statement::execute(THD *thd, String *expanded_query, if (resource_group_switched) mgr_ptr->restore_original_resource_group(thd, src_res_grp, dest_res_grp); - thd->resource_group_ctx()->m_switch_resource_group_str[0] = '\0'; if (ticket != nullptr) mgr_ptr->release_shared_mdl_for_resource_group(thd, ticket); if (cur_ticket != nullptr)