From 1c25787b886aecba4530babf0a46a1dd248df7e6 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 20 Feb 2026 08:39:03 +0000 Subject: [PATCH 1/3] 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(); +} From 7b33b282b1fff773d8b46b90989cd06ba52c2cbc Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Fri, 20 Feb 2026 18:56:44 +0000 Subject: [PATCH 2/3] Fix compilation error in issue5384-t TAP test --- test/tap/tests/issue5384-t.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/tap/tests/issue5384-t.cpp b/test/tap/tests/issue5384-t.cpp index 6c5b8ec2d..306de3792 100644 --- a/test/tap/tests/issue5384-t.cpp +++ b/test/tap/tests/issue5384-t.cpp @@ -68,7 +68,11 @@ int main(int, char**) { // 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); + if (mysql_query_t(admin, q_stats)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin)); + return exit_status(); + } + MYSQL_RES *res = mysql_store_result(admin); MYSQL_ROW row = mysql_fetch_row(res); if (row) { int hg = atoi(row[0]); @@ -93,7 +97,11 @@ int main(int, char**) { run_q(proxy, query); diag("Running on Admin: %s", q_stats); - res = MYSQL_QUERY_T(admin, q_stats); + if (mysql_query_t(admin, q_stats)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin)); + return exit_status(); + } + res = mysql_store_result(admin); row = mysql_fetch_row(res); if (row) { int hg = atoi(row[0]); From 5988468521667ac4ff039d4950081252370ce8e3 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Sun, 22 Feb 2026 12:09:49 +0000 Subject: [PATCH 3/3] Query Processor: re-initialize parser on rewritten queries and enhance test coverage - Fixed Query Processor to re-extract comments and re-compute digest if a query rule rewrites the query. - Enhanced issue5384-t with better regex, robust NULL checks, and teardown logic. - Added pgsql-issue5384-t to provide parity coverage for the PostgreSQL module. - Registered the new test in groups.json. --- lib/Query_Processor.cpp | 8 +- test/tap/groups/groups.json | 1 + test/tap/tests/issue5384-t.cpp | 86 ++++++++++++--- test/tap/tests/pgsql-issue5384-t.cpp | 159 +++++++++++++++++++++++++++ 4 files changed, 234 insertions(+), 20 deletions(-) create mode 100644 test/tap/tests/pgsql-issue5384-t.cpp diff --git a/lib/Query_Processor.cpp b/lib/Query_Processor.cpp index 268ea1214..e0e5314a4 100644 --- a/lib/Query_Processor.cpp +++ b/lib/Query_Processor.cpp @@ -1664,9 +1664,13 @@ __exit_process_mysql_query: } if (sess->mirror==false) { // we process comments only on original queries, not on mirrors - if (qp && qp->first_comment) { + if (qp) { + if (ret->new_query) { + query_parser_free(qp); + query_parser_init(qp, ret->new_query->c_str(), ret->new_query->length(), 0); + } // Process first comment after query rules if configured (values 2 or 3) - if (first_comment_parsing == 2 || first_comment_parsing == 3) { + if (qp->first_comment && (first_comment_parsing == 2 || first_comment_parsing == 3)) { // we have a comment to parse query_parser_first_comment(ret, qp->first_comment); } diff --git a/test/tap/groups/groups.json b/test/tap/groups/groups.json index ef73ba673..181a6a751 100644 --- a/test/tap/groups/groups.json +++ b/test/tap/groups/groups.json @@ -247,6 +247,7 @@ "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-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 index 306de3792..469ec25cd 100644 --- a/test/tap/tests/issue5384-t.cpp +++ b/test/tap/tests/issue5384-t.cpp @@ -6,8 +6,8 @@ * - Verifies that the comment is still parsed even if stripped by a rule. */ -#include -#include +#include +#include #include "command_line.h" #include "mysql.h" @@ -20,7 +20,7 @@ int main(int, char**) { return -1; } - plan(2); + plan(3); MYSQL* admin = init_mysql_conn(cl.host, cl.admin_port, cl.admin_username, cl.admin_password); if (!admin) { @@ -29,6 +29,7 @@ int main(int, char**) { MYSQL* proxy = init_mysql_conn(cl.host, cl.port, cl.username, cl.password); if (!proxy) { + mysql_close(admin); return exit_status(); } @@ -39,8 +40,8 @@ int main(int, char**) { 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)"; + // Rule to strip the comment. Improved regex to be non-greedy and optional trailing space. + 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); @@ -63,7 +64,7 @@ int main(int, char**) { // 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. + run_q(proxy, query); // 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 ?'"; @@ -73,14 +74,18 @@ int main(int, char**) { return exit_status(); } MYSQL_RES *res = mysql_store_result(admin); - 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); + if (res) { + 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 (Test 1)"); + } + mysql_free_result(res); } else { - ok(0, "Failed to find query in stats"); + ok(0, "mysql_store_result returned NULL (Test 1)"); } - 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"; @@ -102,14 +107,59 @@ int main(int, char**) { return exit_status(); } res = mysql_store_result(admin); - 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); + if (res) { + MYSQL_ROW 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 (Test 2)"); + } + mysql_free_result(res); } else { - ok(0, "Failed to find query in stats"); + ok(0, "mysql_store_result returned NULL (Test 2)"); } - mysql_free_result(res); + + diag(" ========== Test 3: Both passes (mode 3) =========="); + // In mode 3, it parses before and after. If stripped, it should still work due to the before pass. + const char *q_set_var3 = "SET mysql-query_processor_first_comment_parsing = 3"; + diag("Running on Admin: %s", q_set_var3); + MYSQL_QUERY_T(admin, q_set_var3); + + 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); + if (mysql_query_t(admin, q_stats)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin)); + return exit_status(); + } + res = mysql_store_result(admin); + if (res) { + MYSQL_ROW row = mysql_fetch_row(res); + if (row) { + int hg = atoi(row[0]); + ok(hg == 1000, "Comment SHOULD have been parsed in the BEFORE pass (mode 3). hg=%d", hg); + } else { + ok(0, "Failed to find query in stats (Test 3)"); + } + mysql_free_result(res); + } else { + ok(0, "mysql_store_result returned NULL (Test 3)"); + } + + // Teardown: restore defaults + diag("Teardown: restoring defaults"); + MYSQL_QUERY_T(admin, "DELETE FROM mysql_query_rules WHERE rule_id=1"); + MYSQL_QUERY_T(admin, "LOAD MYSQL QUERY RULES TO RUNTIME"); + MYSQL_QUERY_T(admin, "SET mysql-query_processor_first_comment_parsing = 2"); + MYSQL_QUERY_T(admin, "LOAD MYSQL VARIABLES TO RUNTIME"); mysql_close(admin); mysql_close(proxy); diff --git a/test/tap/tests/pgsql-issue5384-t.cpp b/test/tap/tests/pgsql-issue5384-t.cpp new file mode 100644 index 000000000..bc79baa0f --- /dev/null +++ b/test/tap/tests/pgsql-issue5384-t.cpp @@ -0,0 +1,159 @@ +/** + * @file pgsql-issue5384-t.cpp + * @brief This test file verifies the functionality of the pgsql-query_processor_first_comment_parsing variable. + * - Sets pgsql-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 +#include +#include +#include +#include +#include +#include "libpq-fe.h" +#include "command_line.h" +#include "tap.h" +#include "utils.h" + +CommandLine cl; + +using PGConnPtr = std::unique_ptr; + +enum ConnType { + ADMIN, + BACKEND +}; + +PGConnPtr createNewConnection(ConnType conn_type, const std::string& options = "", bool with_ssl = false) { + const char* host = (conn_type == BACKEND) ? cl.pgsql_host : cl.pgsql_admin_host; + int port = (conn_type == BACKEND) ? cl.pgsql_port : cl.pgsql_admin_port; + const char* username = (conn_type == BACKEND) ? cl.pgsql_username : cl.admin_username; + const char* password = (conn_type == BACKEND) ? cl.pgsql_password : cl.admin_password; + + std::stringstream ss; + ss << "host=" << host << " port=" << port; + ss << " user=" << username << " password=" << password; + ss << (with_ssl ? " sslmode=require" : " sslmode=disable"); + + if (options.empty() == false) { + ss << " options='" << options << "'"; + } + + PGconn* conn = PQconnectdb(ss.str().c_str()); + if (PQstatus(conn) != CONNECTION_OK) { + diag("Connection failed to '%s': %s", + (conn_type == BACKEND ? "Backend" : "Admin"), + PQerrorMessage(conn)); + PQfinish(conn); + return PGConnPtr(nullptr, &PQfinish); + } + return PGConnPtr(conn, &PQfinish); +} + +int get_query_hg_from_stats(PGconn* admin, const char* query_digest_text) { + std::stringstream ss; + ss << "SELECT destination_hostgroup FROM stats_pgsql_query_digest WHERE digest_text='" << query_digest_text << "'"; + PGresult* res = PQexec(admin, ss.str().c_str()); + if (PQresultStatus(res) != PGRES_TUPLES_OK) { + diag("Failed to get query stats: %s", PQerrorMessage(admin)); + PQclear(res); + return -1; + } + int hg = -1; + if (PQntuples(res) > 0) { + char *val = PQgetvalue(res, 0, 0); + if (val) { + hg = atoi(val); + } + } + PQclear(res); + return hg; +} + +int main(int argc, char** argv) { + if (cl.getEnv()) { + diag("Failed to get the required environmental variables."); + return -1; + } + + plan(3); + + PGConnPtr admin = createNewConnection(ADMIN); + if (!admin) { + BAIL_OUT("Failed to connect to admin interface"); + return exit_status(); + } + + PGConnPtr proxy = createNewConnection(BACKEND); + if (!proxy) { + BAIL_OUT("Failed to connect to proxy"); + return exit_status(); + } + + diag(" ========== Test 1: Default behavior (parsed after rules) =========="); + PQclear(PQexec(admin.get(), "DELETE FROM pgsql_query_rules")); + + // Rule to strip the comment. PostgreSQL comments can be /* */ or -- + // Note: the regex needs to match the comment format used in the test query. + PQclear(PQexec(admin.get(), "INSERT INTO pgsql_query_rules (rule_id, active, match_pattern, replace_pattern, apply) VALUES (1, 1, '/\\\\*.*?\\\\*/ ?', '', 1)")); + PQclear(PQexec(admin.get(), "LOAD PGSQL QUERY RULES TO RUNTIME")); + + PQclear(PQexec(admin.get(), "SET pgsql-query_processor_first_comment_parsing = 2")); + PQclear(PQexec(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME")); + + PQclear(PQexec(admin.get(), "TRUNCATE stats_pgsql_query_digest")); + + const char *query = "/*+ hostgroup=1000 */ SELECT 1"; + diag("Running on Proxy: %s", query); + PQclear(PQexec(proxy.get(), query)); + + int hg = get_query_hg_from_stats(admin.get(), "SELECT ?"); + if (hg != -1) { + 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 (Test 1)"); + } + + diag(" ========== Test 2: New behavior (parsed before rules) =========="); + PQclear(PQexec(admin.get(), "SET pgsql-query_processor_first_comment_parsing = 1")); + PQclear(PQexec(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME")); + PQclear(PQexec(admin.get(), "TRUNCATE stats_pgsql_query_digest")); + + diag("Running on Proxy: %s", query); + PQclear(PQexec(proxy.get(), query)); + + hg = get_query_hg_from_stats(admin.get(), "SELECT ?"); + if (hg != -1) { + 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 (Test 2)"); + } + + diag(" ========== Test 3: Both passes (mode 3) =========="); + PQclear(PQexec(admin.get(), "SET pgsql-query_processor_first_comment_parsing = 3")); + PQclear(PQexec(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME")); + PQclear(PQexec(admin.get(), "TRUNCATE stats_pgsql_query_digest")); + + diag("Running on Proxy: %s", query); + PQclear(PQexec(proxy.get(), query)); + + hg = get_query_hg_from_stats(admin.get(), "SELECT ?"); + if (hg != -1) { + ok(hg == 1000, "Comment SHOULD have been parsed in the BEFORE pass (mode 3). hg=%d", hg); + } else { + ok(0, "Failed to find query in stats (Test 3)"); + } + + // Teardown + diag("Teardown: restoring defaults"); + PQclear(PQexec(admin.get(), "DELETE FROM pgsql_query_rules WHERE rule_id=1")); + PQclear(PQexec(admin.get(), "LOAD PGSQL QUERY RULES TO RUNTIME")); + PQclear(PQexec(admin.get(), "SET pgsql-query_processor_first_comment_parsing = 2")); + PQclear(PQexec(admin.get(), "LOAD PGSQL VARIABLES TO RUNTIME")); + + return exit_status(); +}