From 50243ef283370eb5d448a75490bca05840e55563 Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Wed, 30 Jul 2025 14:43:00 +0500 Subject: [PATCH] Corrected RESET/DEFAULT logic * Now will actually reset value to server default (represented by nullptr value), if that parameter is not set in startup parameter. * Default parameter values (pgsql_default_*) will now be set only for critical parameters. --- include/PgSQL_Thread.h | 2 +- include/proxysql_structs.h | 4 +-- lib/PgSQL_Connection.cpp | 4 +-- lib/PgSQL_Session.cpp | 66 +++++++++++++++++++++++--------------- lib/PgSQL_Thread.cpp | 42 +++++++----------------- 5 files changed, 56 insertions(+), 62 deletions(-) diff --git a/include/PgSQL_Thread.h b/include/PgSQL_Thread.h index 2e8f0a8cd..dc3f951e4 100644 --- a/include/PgSQL_Thread.h +++ b/include/PgSQL_Thread.h @@ -957,7 +957,7 @@ public: char* ldap_user_variable; char* add_ldap_user_comment; char* default_session_track_gtids; - char* default_variables[PGSQL_NAME_LAST_HIGH_WM]; + char* default_variables[PGSQL_NAME_LAST_LOW_WM]; char* firewall_whitelist_errormsg; #ifdef DEBUG bool session_debug; diff --git a/include/proxysql_structs.h b/include/proxysql_structs.h index f986ce39f..83b9b6b96 100644 --- a/include/proxysql_structs.h +++ b/include/proxysql_structs.h @@ -1192,7 +1192,7 @@ __thread int pgsql_thread___query_cache_soft_ttl_pct; __thread int pgsql_thread___query_cache_handle_warnings; __thread bool pgsql_thread___session_idle_show_processlist; -__thread char* pgsql_thread___default_variables[PGSQL_NAME_LAST_HIGH_WM]; +__thread char* pgsql_thread___default_variables[PGSQL_NAME_LAST_LOW_WM]; __thread int pgsql_thread___handle_unknown_charset; //--------------------------- @@ -1497,7 +1497,7 @@ extern __thread int pgsql_thread___query_cache_soft_ttl_pct; extern __thread int pgsql_thread___query_cache_handle_warnings; extern __thread bool pgsql_thread___session_idle_show_processlist; -extern __thread char* pgsql_thread___default_variables[PGSQL_NAME_LAST_HIGH_WM]; +extern __thread char* pgsql_thread___default_variables[PGSQL_NAME_LAST_LOW_WM]; extern __thread int pgsql_thread___handle_unknown_charset; //--------------------------- diff --git a/lib/PgSQL_Connection.cpp b/lib/PgSQL_Connection.cpp index cac19a4af..c1ef2085d 100644 --- a/lib/PgSQL_Connection.cpp +++ b/lib/PgSQL_Connection.cpp @@ -2208,8 +2208,8 @@ std::pair PgSQL_Connection::get_startup_parameter_and_has assert(startup_parameters[idx]); return { startup_parameters[idx], startup_parameters_hash[idx] }; } - // fall back to thread-specific default - return { pgsql_thread___default_variables[idx], 0 }; + assert(!(idx < PGSQL_NAME_LAST_LOW_WM)); + return { "", 0}; } void PgSQL_Connection::copy_pgsql_variables_to_startup_parameters(bool copy_only_critical_param) { diff --git a/lib/PgSQL_Session.cpp b/lib/PgSQL_Session.cpp index 3863d2045..6f2854082 100644 --- a/lib/PgSQL_Session.cpp +++ b/lib/PgSQL_Session.cpp @@ -4210,15 +4210,32 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C int idx = PGSQL_NAME_LAST_HIGH_WM; for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { + // skip low water mark + if (i == PGSQL_NAME_LAST_LOW_WM) continue; + if (variable_name_exists(pgsql_tracked_variables[i], var.c_str()) == true) { idx = i; break; } } if (idx != PGSQL_NAME_LAST_HIGH_WM) { - + uint32_t current_hash = pgsql_variables.client_get_hash(this, idx); if ((value1.size() == sizeof("DEFAULT") - 1) && strncasecmp(value1.c_str(), (char*)"DEFAULT",sizeof("DEFAULT")-1) == 0) { - std::tie(value1, std::ignore) = client_myds->myconn->get_startup_parameter_and_hash((enum pgsql_variable_name)idx); + auto [value, hash] = client_myds->myconn->get_startup_parameter_and_hash((enum pgsql_variable_name)idx); + if (hash == 0) { + if (current_hash != 0) { + proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Resetting connection variable %s to DEFAULT\n", var.c_str()); + pgsql_variables.client_reset_value(this, idx, true); + } + client_myds->DSS = STATE_QUERY_SENT_NET; + unsigned int nTrx = NumActiveTransactions(); + const char trx_state = (nTrx ? 'T' : 'I'); + client_myds->myprot.generate_ok_packet(true, true, NULL, 0, dig, trx_state, NULL, param_status); + RequestEnd(NULL); + l_free(pkt->size, pkt->ptr); + return true; + } + value1 = value; } char* transformed_value = nullptr; @@ -4259,10 +4276,10 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C return true; } } - - proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Changing connection %s to %s\n", var.c_str(), value1.c_str()); + uint32_t var_hash_int = SpookyHash::Hash32(value1.c_str(), value1.length(), 10); - if (pgsql_variables.client_get_hash(this, idx) != var_hash_int) { + if (current_hash != var_hash_int) { + proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Changing connection %s to %s\n", var.c_str(), value1.c_str()); if (!pgsql_variables.client_set_value(this, idx, value1.c_str(), true)) { return false; } @@ -4445,8 +4462,7 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C std::string value1 = *values; proxy_debug(PROXY_DEBUG_MYSQL_COM, 5, "Processing SET %s value %s\n", var.c_str(), value1.c_str()); #endif // DEBUG - } - else { + } else { // At this point the variable is unknown to us, or it's a user variable // prefixed by '@', in both cases, we should fail to parse. We don't // fail inmediately so we can anyway keep track of the other variables @@ -4638,10 +4654,10 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C const char* name = pgsql_tracked_variables[idx].set_variable_name; auto [value, hash] = client_myds->myconn->get_startup_parameter_and_hash((enum pgsql_variable_name)idx); - - proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Changing connection %s to %s\n", name, value); - uint32_t var_hash_int = ((hash != 0) ? hash : SpookyHash::Hash32(value, strlen(value), 10)); - if (pgsql_variables.client_get_hash(this, idx) != var_hash_int) { + // hash can never be 0 for critical variables + uint32_t current_hash = pgsql_variables.client_get_hash(this, idx); + if (current_hash != hash) { + proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Changing connection %s to %s\n", name, value); if (!pgsql_variables.client_set_value(this, idx, value, false)) { return false; } @@ -4652,21 +4668,19 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C } for (int idx : client_myds->myconn->dynamic_variables_idx) { - assert(idx < PGSQL_NAME_LAST_HIGH_WM); const char* name = pgsql_tracked_variables[idx].set_variable_name; auto [value, hash] = client_myds->myconn->get_startup_parameter_and_hash((enum pgsql_variable_name)idx); - proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Changing connection %s to %s\n", name, value); - uint32_t var_hash_int = ((hash != 0) ? hash : SpookyHash::Hash32(value, strlen(value), 10)); - if (pgsql_variables.client_get_hash(this, idx) != var_hash_int) { + uint32_t current_hash = pgsql_variables.client_get_hash(this, idx); + if (hash == 0 && current_hash != 0) { + proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Resetting connection variable %s to DEFAULT\n", name); + pgsql_variables.client_reset_value(this, idx, false); + } else if (hash != 0 && current_hash != hash) { + proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Changing connection %s to %s\n", name, value); if (!pgsql_variables.client_set_value(this, idx, value, false)) { return false; } - if (IS_PGTRACKED_VAR_OPTION_SET_PARAM_STATUS(pgsql_tracked_variables[idx])) { - param_status.emplace_back(name, value); - } } } - client_myds->myconn->reorder_dynamic_variables_idx(); } else if (std::find(pgsql_variables.ignore_vars.begin(), pgsql_variables.ignore_vars.end(), nq) != pgsql_variables.ignore_vars.end()) { @@ -4675,10 +4689,8 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C proxy_debug(PROXY_DEBUG_MYSQL_COM, 5, "Processing RESET %s\n", nq.c_str()); #endif // DEBUG } else { - int idx = PGSQL_NAME_LAST_HIGH_WM; for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { - if (i == PGSQL_NAME_LAST_LOW_WM) continue; @@ -4687,18 +4699,20 @@ bool PgSQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C break; } } - if (idx != PGSQL_NAME_LAST_HIGH_WM) { const char* name = pgsql_tracked_variables[idx].set_variable_name; auto [value, hash] = client_myds->myconn->get_startup_parameter_and_hash((enum pgsql_variable_name)idx); - - uint32_t var_hash_int = ((hash != 0) ? hash : SpookyHash::Hash32(value, strlen(value), 10)); - if (pgsql_variables.client_get_hash(this, idx) != var_hash_int) { + uint32_t current_hash = pgsql_variables.client_get_hash(this, idx); + // Reset to default if hash is zero, means startup parameter is not set + if (hash == 0 && current_hash != 0) { + proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Resetting connection variable %s to DEFAULT\n", name); + pgsql_variables.client_reset_value(this, idx, true); + } else if (hash != 0 && current_hash != hash) { + proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Changing connection %s to %s\n", name, value); if (!pgsql_variables.client_set_value(this, idx, value, true)) { return false; } if (IS_PGTRACKED_VAR_OPTION_SET_PARAM_STATUS(pgsql_tracked_variables[idx])) { - proxy_debug(PROXY_DEBUG_MYSQL_COM, 8, "Changing connection %s to %s\n", name, value); param_status.emplace_back(name, value); } } diff --git a/lib/PgSQL_Thread.cpp b/lib/PgSQL_Thread.cpp index f6f3c6a53..6c0b39f4e 100644 --- a/lib/PgSQL_Thread.cpp +++ b/lib/PgSQL_Thread.cpp @@ -1020,7 +1020,7 @@ PgSQL_Threads_Handler::PgSQL_Threads_Handler() { variables.init_connect = NULL; variables.ldap_user_variable = NULL; variables.add_ldap_user_comment = NULL; - for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { + for (int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { variables.default_variables[i] = strdup(pgsql_tracked_variables[i].default_value); } variables.default_session_track_gtids = strdup((char*)MYSQL_DEFAULT_SESSION_TRACK_GTIDS); @@ -1289,9 +1289,7 @@ char* PgSQL_Threads_Handler::get_variable_string(char* name) { } } if (!strncmp(name, "default_", 8)) { - for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { - if (i == PGSQL_NAME_LAST_LOW_WM) - continue; + for (int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { char buf[128]; sprintf(buf, "default_%s", pgsql_tracked_variables[i].internal_variable_name); if (!strcmp(name, buf)) { @@ -1422,9 +1420,7 @@ char* PgSQL_Threads_Handler::get_variable(char* name) { // this is the public fu } if (strlen(name) > 8) { if (strncmp(name, "default_", 8) == 0) { - for (unsigned int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { - if (i == PGSQL_NAME_LAST_LOW_WM) - continue; + for (int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { size_t var_len = strlen(pgsql_tracked_variables[i].internal_variable_name); if (strlen(name) == (var_len + 8)) { if (!strncmp(name + 8, pgsql_tracked_variables[i].internal_variable_name, var_len)) { @@ -1802,11 +1798,7 @@ bool PgSQL_Threads_Handler::set_variable(char* name, const char* value) { // thi } if (!strncmp(name, "default_", 8)) { - for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { - - if (i == PGSQL_NAME_LAST_LOW_WM) - continue; - + for (int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { char buf[128]; sprintf(buf, "default_%s", pgsql_tracked_variables[i].internal_variable_name); if (!strcmp(name, buf)) { @@ -1842,7 +1834,6 @@ bool PgSQL_Threads_Handler::set_variable(char* name, const char* value) { // thi } } - if (!strcasecmp(name, "keep_multiplexing_variables")) { if (vallen) { free(variables.keep_multiplexing_variables); @@ -2249,17 +2240,10 @@ char** PgSQL_Threads_Handler::get_variables_list() { const size_t l = sizeof(pgsql_thread_variables_names) / sizeof(char*); unsigned int i; - size_t ltv = 0; - for (i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { - if (i == PGSQL_NAME_LAST_LOW_WM) - continue; - ltv++; - } + const size_t ltv = PGSQL_NAME_LAST_LOW_WM; char** ret = (char**)malloc(sizeof(char*) * (l + ltv)); // not adding + 1 because pgsql_thread_variables_names is already NULL terminated size_t fv = 0; - for (i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { - if (i == PGSQL_NAME_LAST_LOW_WM) - continue; + for (i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { char* m = (char*)malloc(strlen(pgsql_tracked_variables[i].internal_variable_name) + 1 + strlen((char*)"default_")); sprintf(m, "default_%s", pgsql_tracked_variables[i].internal_variable_name); ret[fv] = m; @@ -2279,9 +2263,7 @@ char** PgSQL_Threads_Handler::get_variables_list() { bool PgSQL_Threads_Handler::has_variable(const char* name) { if (strlen(name) > 8) { if (strncmp(name, "default_", 8) == 0) { - for (unsigned int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { - if (i == PGSQL_NAME_LAST_LOW_WM) - continue; + for (unsigned int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { size_t var_len = strlen(pgsql_tracked_variables[i].internal_variable_name); if (strlen(name) == (var_len + 8)) { if (!strncmp(name + 8, pgsql_tracked_variables[i].internal_variable_name, var_len)) { @@ -2672,7 +2654,7 @@ PgSQL_Threads_Handler::~PgSQL_Threads_Handler() { if (variables.ssl_p2s_crl) free(variables.ssl_p2s_crl); if (variables.ssl_p2s_crlpath) free(variables.ssl_p2s_crlpath); - for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { + for (int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { if (variables.default_variables[i]) { free(variables.default_variables[i]); variables.default_variables[i] = NULL; @@ -2801,7 +2783,7 @@ PgSQL_Thread::~PgSQL_Thread() { if (pgsql_thread___server_version) { free(pgsql_thread___server_version); pgsql_thread___server_version = NULL; } if (pgsql_thread___server_encoding) { free(pgsql_thread___server_encoding); pgsql_thread___server_encoding = NULL; } - for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { + for (int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { if (pgsql_thread___default_variables[i]) { free(pgsql_thread___default_variables[i]); pgsql_thread___default_variables[i] = NULL; @@ -3926,9 +3908,7 @@ void PgSQL_Thread::refresh_variables() { mysql_thread___default_session_track_gtids = GloPTH->get_variable_string((char*)"default_session_track_gtids"); */ - for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { - if (i == PGSQL_NAME_LAST_LOW_WM) - continue; + for (int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { if (pgsql_thread___default_variables[i]) { free(pgsql_thread___default_variables[i]); pgsql_thread___default_variables[i] = NULL; @@ -4091,7 +4071,7 @@ PgSQL_Thread::PgSQL_Thread() { variables.stats_time_query_processor = false; variables.query_cache_stores_empty_result = true; - for (int i = 0; i < PGSQL_NAME_LAST_HIGH_WM; i++) { + for (int i = 0; i < PGSQL_NAME_LAST_LOW_WM; i++) { pgsql_thread___default_variables[i] = NULL; } shutdown = 0;