From 1c25787b886aecba4530babf0a46a1dd248df7e6 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 20 Feb 2026 08:39:03 +0000 Subject: [PATCH] Introduce {mysql,pgsql}-query_processor_first_comment_parsing variable (#5384) This universal fix allows controlling when the first comment of a query is processed relative to the query rules. By setting this variable to 1, ProxySQL-specific annotations (like GTID) can be parsed immediately, allowing subsequent query rules to strip them from the query string. This prevents prepared statement cache bloat and improves backend statement reuse when annotations with unique literals are used. --- include/MySQL_Thread.h | 8 +++ include/PgSQL_Thread.h | 8 +++ include/proxysql_structs.h | 4 ++ lib/MySQL_Thread.cpp | 4 ++ lib/PgSQL_Thread.cpp | 4 ++ lib/Query_Processor.cpp | 17 ++++- test/tap/groups/groups.json | 1 + test/tap/tests/issue5384-t.cpp | 110 +++++++++++++++++++++++++++++++++ 8 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 test/tap/tests/issue5384-t.cpp diff --git a/include/MySQL_Thread.h b/include/MySQL_Thread.h index c059b1690..8548eb7a4 100644 --- a/include/MySQL_Thread.h +++ b/include/MySQL_Thread.h @@ -520,6 +520,14 @@ class MySQL_Threads_Handler int default_query_delay; int default_query_timeout; int query_processor_iterations; + /** + * @brief Defines when the first comment of a query needs to be processed. + * 0 : comment ignored + * 1 : comment processed before the query rules + * 2 : comment processed after the query rules (default behavior) + * 3 : comment processed before and after the query rules + */ + int query_processor_first_comment_parsing; int query_processor_regex; int set_query_lock_on_hostgroup; int set_parser_algorithm; diff --git a/include/PgSQL_Thread.h b/include/PgSQL_Thread.h index 72a9361fc..28ea1d39b 100644 --- a/include/PgSQL_Thread.h +++ b/include/PgSQL_Thread.h @@ -1006,6 +1006,14 @@ public: int default_query_delay; int default_query_timeout; int query_processor_iterations; + /** + * @brief Defines when the first comment of a query needs to be processed. + * 0 : comment ignored + * 1 : comment processed before the query rules + * 2 : comment processed after the query rules (default behavior) + * 3 : comment processed before and after the query rules + */ + int query_processor_first_comment_parsing; int query_processor_regex; int set_query_lock_on_hostgroup; int set_parser_algorithm; diff --git a/include/proxysql_structs.h b/include/proxysql_structs.h index 6ae5fc7dd..d407a25ad 100644 --- a/include/proxysql_structs.h +++ b/include/proxysql_structs.h @@ -1187,6 +1187,7 @@ __thread int pgsql_thread___eventslog_rate_limit; __thread char* pgsql_thread___firewall_whitelist_errormsg; __thread bool pgsql_thread___firewall_whitelist_enabled; __thread int pgsql_thread___query_processor_iterations; +__thread int pgsql_thread___query_processor_first_comment_parsing; __thread int pgsql_thread___query_processor_regex; __thread bool pgsql_thread___monitor_enabled; @@ -1267,6 +1268,7 @@ __thread int mysql_thread___connect_timeout_client; __thread int mysql_thread___connect_timeout_server; __thread int mysql_thread___connect_timeout_server_max; __thread int mysql_thread___query_processor_iterations; +__thread int mysql_thread___query_processor_first_comment_parsing; __thread int mysql_thread___query_processor_regex; __thread int mysql_thread___set_query_lock_on_hostgroup; __thread int mysql_thread___set_parser_algorithm; @@ -1504,6 +1506,7 @@ extern __thread int pgsql_thread___eventslog_rate_limit; extern __thread char* pgsql_thread___firewall_whitelist_errormsg; extern __thread bool pgsql_thread___firewall_whitelist_enabled; extern __thread int pgsql_thread___query_processor_iterations; +extern __thread int pgsql_thread___query_processor_first_comment_parsing; extern __thread int pgsql_thread___query_processor_regex; extern __thread bool pgsql_thread___monitor_enabled; @@ -1584,6 +1587,7 @@ extern __thread int mysql_thread___connect_timeout_client; extern __thread int mysql_thread___connect_timeout_server; extern __thread int mysql_thread___connect_timeout_server_max; extern __thread int mysql_thread___query_processor_iterations; +extern __thread int mysql_thread___query_processor_first_comment_parsing; extern __thread int mysql_thread___query_processor_regex; extern __thread int mysql_thread___set_query_lock_on_hostgroup; extern __thread int mysql_thread___set_parser_algorithm; diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp index 4c7c8a0b1..496ac46cd 100644 --- a/lib/MySQL_Thread.cpp +++ b/lib/MySQL_Thread.cpp @@ -456,6 +456,7 @@ static char * mysql_thread_variables_names[]= { (char *)"default_query_delay", (char *)"default_query_timeout", (char *)"query_processor_iterations", + (char *)"query_processor_first_comment_parsing", (char *)"query_processor_regex", (char *)"set_query_lock_on_hostgroup", (char *)"set_parser_algorithm", @@ -1329,6 +1330,7 @@ MySQL_Threads_Handler::MySQL_Threads_Handler() { variables.default_query_delay=0; variables.default_query_timeout=24*3600*1000; variables.query_processor_iterations=0; + variables.query_processor_first_comment_parsing=2; variables.query_processor_regex=1; variables.set_query_lock_on_hostgroup=1; variables.set_parser_algorithm=2; // before 2.6.0 this was 1 @@ -2589,6 +2591,7 @@ char ** MySQL_Threads_Handler::get_variables_list() { VariablesPointers_int["query_digests_max_query_length"] = make_tuple(&variables.query_digests_max_query_length, 16, 1*1024*1024, false); VariablesPointers_int["query_rules_fast_routing_algorithm"] = make_tuple(&variables.query_rules_fast_routing_algorithm, 1, 2, false); VariablesPointers_int["query_processor_iterations"] = make_tuple(&variables.query_processor_iterations, 0, 1000*1000, false); + VariablesPointers_int["query_processor_first_comment_parsing"] = make_tuple(&variables.query_processor_first_comment_parsing, 0, 3, false); VariablesPointers_int["query_processor_regex"] = make_tuple(&variables.query_processor_regex, 1, 2, false); VariablesPointers_int["query_retries_on_failure"] = make_tuple(&variables.query_retries_on_failure, 0, 1000, false); VariablesPointers_int["set_query_lock_on_hostgroup"] = make_tuple(&variables.set_query_lock_on_hostgroup, 0, 1, false); @@ -4553,6 +4556,7 @@ void MySQL_Thread::refresh_variables() { REFRESH_VARIABLE_INT(default_query_delay); REFRESH_VARIABLE_INT(default_query_timeout); REFRESH_VARIABLE_INT(query_processor_iterations); + REFRESH_VARIABLE_INT(query_processor_first_comment_parsing); REFRESH_VARIABLE_INT(query_processor_regex); REFRESH_VARIABLE_INT(set_query_lock_on_hostgroup); REFRESH_VARIABLE_INT(set_parser_algorithm); diff --git a/lib/PgSQL_Thread.cpp b/lib/PgSQL_Thread.cpp index c03a40e6e..80544d0c0 100644 --- a/lib/PgSQL_Thread.cpp +++ b/lib/PgSQL_Thread.cpp @@ -397,6 +397,7 @@ static char* pgsql_thread_variables_names[] = { (char*)"default_query_delay", (char*)"default_query_timeout", (char*)"query_processor_iterations", + (char*)"query_processor_first_comment_parsing", (char*)"query_processor_regex", (char*)"set_query_lock_on_hostgroup", (char*)"set_parser_algorithm", @@ -1115,6 +1116,7 @@ PgSQL_Threads_Handler::PgSQL_Threads_Handler() { variables.default_query_delay = 0; variables.default_query_timeout = 24 * 3600 * 1000; variables.query_processor_iterations = 0; + variables.query_processor_first_comment_parsing = 2; variables.query_processor_regex = 1; variables.set_query_lock_on_hostgroup = 1; variables.set_parser_algorithm = 2; // before 2.6.0 this was 1 @@ -2275,6 +2277,7 @@ char** PgSQL_Threads_Handler::get_variables_list() { VariablesPointers_int["query_digests_max_query_length"] = make_tuple(&variables.query_digests_max_query_length, 16, 1 * 1024 * 1024, false); VariablesPointers_int["query_rules_fast_routing_algorithm"] = make_tuple(&variables.query_rules_fast_routing_algorithm, 1, 2, false); VariablesPointers_int["query_processor_iterations"] = make_tuple(&variables.query_processor_iterations, 0, 1000 * 1000, false); + VariablesPointers_int["query_processor_first_comment_parsing"] = make_tuple(&variables.query_processor_first_comment_parsing, 0, 3, false); VariablesPointers_int["query_processor_regex"] = make_tuple(&variables.query_processor_regex, 1, 2, false); VariablesPointers_int["query_retries_on_failure"] = make_tuple(&variables.query_retries_on_failure, 0, 1000, false); VariablesPointers_int["set_query_lock_on_hostgroup"] = make_tuple(&variables.set_query_lock_on_hostgroup, 0, 1, false); @@ -3980,6 +3983,7 @@ void PgSQL_Thread::refresh_variables() { pgsql_thread___query_digests_max_digest_length = GloPTH->get_variable_int((char*)"query_digests_max_digest_length"); pgsql_thread___query_digests_max_query_length = GloPTH->get_variable_int((char*)"query_digests_max_query_length"); pgsql_thread___query_processor_iterations = GloPTH->get_variable_int((char*)"query_processor_iterations"); + pgsql_thread___query_processor_first_comment_parsing = GloPTH->get_variable_int((char*)"query_processor_first_comment_parsing"); pgsql_thread___query_processor_regex = GloPTH->get_variable_int((char*)"query_processor_regex"); pgsql_thread___query_cache_size_MB = GloPTH->get_variable_int((char*)"query_cache_size_MB"); diff --git a/lib/Query_Processor.cpp b/lib/Query_Processor.cpp index b02258b97..268ea1214 100644 --- a/lib/Query_Processor.cpp +++ b/lib/Query_Processor.cpp @@ -1359,6 +1359,16 @@ Query_Processor_Output* Query_Processor::process_query(TypeSession* flagIN=sess->next_query_flagIN; } int reiterate=GET_THREAD_VARIABLE(query_processor_iterations); + int first_comment_parsing=GET_THREAD_VARIABLE(query_processor_first_comment_parsing); + if (sess->mirror==false) { // we process comments only on original queries, not on mirrors + if (qp && qp->first_comment) { + // Process first comment before query rules if configured (values 1 or 3) + if (first_comment_parsing == 1 || first_comment_parsing == 3) { + // we have a comment to parse + query_parser_first_comment(ret, qp->first_comment); + } + } + } if (sess->mirror==true) { // we are into a mirror session // we immediately set a destination_hostgroup @@ -1655,8 +1665,11 @@ __exit_process_mysql_query: if (sess->mirror==false) { // we process comments only on original queries, not on mirrors if (qp && qp->first_comment) { - // we have a comment to parse - query_parser_first_comment(ret, qp->first_comment); + // Process first comment after query rules if configured (values 2 or 3) + if (first_comment_parsing == 2 || first_comment_parsing == 3) { + // we have a comment to parse + query_parser_first_comment(ret, qp->first_comment); + } } } if (GET_THREAD_VARIABLE(firewall_whitelist_enabled)) { diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index 9a52e1861..ef73ba673 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -246,6 +246,7 @@ "test_ssl_fast_forward-3_libmariadb-t": [ "default-g4", "mysql-auto_increment_delay_multiplex=0-g4", "mysql-multiplexing=false-g4", "mysql-query_digests=0-g4", "mysql-query_digests_keep_comment=1-g4" ], "test_ssl_fast_forward-3_libmysql-t": [ "default-g4", "mysql-auto_increment_delay_multiplex=0-g4", "mysql-multiplexing=false-g4", "mysql-query_digests=0-g4", "mysql-query_digests_keep_comment=1-g4" ], "test_ignore_min_gtid-t": [ "default-g4", "mysql-auto_increment_delay_multiplex=0-g4", "mysql-multiplexing=false-g4", "mysql-query_digests=0-g4", "mysql-query_digests_keep_comment=1-g4" ], + "issue5384-t": [ "default-g4", "mysql-auto_increment_delay_multiplex=0-g4", "mysql-multiplexing=false-g4", "mysql-query_digests=0-g4", "mysql-query_digests_keep_comment=1-g4" ], "pgsql-query_digests_stages_test-t": [ "default-g4", "mysql-auto_increment_delay_multiplex=0-g4", "mysql-multiplexing=false-g4", "mysql-query_digests=0-g4", "mysql-query_digests_keep_comment=1-g4" ], "pgsql_admin_metacmds-t": [ "default-g4", "mysql-auto_increment_delay_multiplex=0-g4", "mysql-multiplexing=false-g4", "mysql-query_digests=0-g4", "mysql-query_digests_keep_comment=1-g4" ], "pgsql-monitor_ssl_connections_test-t": [ "default-g4", "mysql-auto_increment_delay_multiplex=0-g4", "mysql-multiplexing=false-g4", "mysql-query_digests=0-g4", "mysql-query_digests_keep_comment=1-g4" ], diff --git a/test/tap/tests/issue5384-t.cpp b/test/tap/tests/issue5384-t.cpp new file mode 100644 index 000000000..6c5b8ec2d --- /dev/null +++ b/test/tap/tests/issue5384-t.cpp @@ -0,0 +1,110 @@ +/** + * @file issue5384-t.cpp + * @brief This test file verifies the functionality of the mysql-query_processor_first_comment_parsing variable. + * - Sets mysql-query_processor_first_comment_parsing=1 (before rules) + * - Sets a rule to strip the comment. + * - Verifies that the comment is still parsed even if stripped by a rule. + */ + +#include +#include + +#include "command_line.h" +#include "mysql.h" +#include "tap.h" +#include "utils.h" + +int main(int, char**) { + CommandLine cl; + if (cl.getEnv()) { + return -1; + } + + plan(2); + + MYSQL* admin = init_mysql_conn(cl.host, cl.admin_port, cl.admin_username, cl.admin_password); + if (!admin) { + return exit_status(); + } + + MYSQL* proxy = init_mysql_conn(cl.host, cl.port, cl.username, cl.password); + if (!proxy) { + return exit_status(); + } + + diag(" ========== Test 1: Default behavior (parsed after rules) =========="); + // By default (2), comment is parsed AFTER rules. + // If a rule strips the comment, it won't be parsed. + const char *q_del_rules = "DELETE FROM mysql_query_rules"; + diag("Running on Admin: %s", q_del_rules); + MYSQL_QUERY_T(admin, q_del_rules); + + // Rule to strip the comment + const char *q_ins_rule = "INSERT INTO mysql_query_rules (rule_id, active, match_pattern, replace_pattern, apply) VALUES (1, 1, '/\\\\*.*\\\\*/ ', '', 1)"; + diag("Running on Admin: %s", q_ins_rule); + MYSQL_QUERY_T(admin, q_ins_rule); + + const char *q_load_rules = "LOAD MYSQL QUERY RULES TO RUNTIME"; + diag("Running on Admin: %s", q_load_rules); + MYSQL_QUERY_T(admin, q_load_rules); + + const char *q_set_var = "SET mysql-query_processor_first_comment_parsing = 2"; + diag("Running on Admin: %s", q_set_var); + MYSQL_QUERY_T(admin, q_set_var); + + const char *q_load_vars = "LOAD MYSQL VARIABLES TO RUNTIME"; + diag("Running on Admin: %s", q_load_vars); + MYSQL_QUERY_T(admin, q_load_vars); + + const char *q_truncate = "TRUNCATE stats_mysql_query_digest"; + diag("Running on Admin: %s", q_truncate); + MYSQL_QUERY_T(admin, q_truncate); + + // We use hostgroup=1000 which likely doesn't exist or is different from default + const char *query = "/*+ hostgroup=1000 */ SELECT 1"; + diag("Running on Proxy: %s", query); + run_q(proxy, query); // This might fail if HG 1000 doesn't exist, but ProxySQL should try to route it. + + // Check stats to see if hostgroup 1000 was used + const char *q_stats = "SELECT destination_hostgroup FROM stats_mysql_query_digest WHERE digest_text='SELECT ?'"; + diag("Running on Admin: %s", q_stats); + MYSQL_RES *res = MYSQL_QUERY_T(admin, q_stats); + MYSQL_ROW row = mysql_fetch_row(res); + if (row) { + int hg = atoi(row[0]); + ok(hg != 1000, "Comment should NOT have been parsed because it was stripped by rule. hg=%d", hg); + } else { + ok(0, "Failed to find query in stats"); + } + mysql_free_result(res); + + diag(" ========== Test 2: New behavior (parsed before rules) =========="); + const char *q_set_var2 = "SET mysql-query_processor_first_comment_parsing = 1"; + diag("Running on Admin: %s", q_set_var2); + MYSQL_QUERY_T(admin, q_set_var2); + + diag("Running on Admin: %s", q_load_vars); + MYSQL_QUERY_T(admin, q_load_vars); + + diag("Running on Admin: %s", q_truncate); + MYSQL_QUERY_T(admin, q_truncate); + + diag("Running on Proxy: %s", query); + run_q(proxy, query); + + diag("Running on Admin: %s", q_stats); + res = MYSQL_QUERY_T(admin, q_stats); + row = mysql_fetch_row(res); + if (row) { + int hg = atoi(row[0]); + ok(hg == 1000, "Comment SHOULD have been parsed BEFORE it was stripped by rule. hg=%d", hg); + } else { + ok(0, "Failed to find query in stats"); + } + mysql_free_result(res); + + mysql_close(admin); + mysql_close(proxy); + + return exit_status(); +}