From dc10bee4d54fa35f95a22853edd4ca6feb0cc84a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= <rene.cannao@gmail.com>
Date: Wed, 24 Feb 2021 21:52:37 +0100
Subject: [PATCH] Adding some comments and a quick exit on
 handler_special_queries for locked_on_hostgroup

---
 lib/MySQL_Session.cpp                            | 16 ++++++++++++----
 ...est_3317-lock_hostgroup_special_queries-t.cpp |  2 ++
 test/tap/tests/setparser_test.cpp                |  1 +
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp
index 818852e00..56ba54163 100644
--- a/lib/MySQL_Session.cpp
+++ b/lib/MySQL_Session.cpp
@@ -1179,7 +1179,15 @@ bool MySQL_Session::handler_special_queries(PtrSize_t *pkt) {
 		l_free(pkt->size,pkt->ptr);
 		return true;
 	}
-	if ((locked_on_hostgroup == -1) && (pkt->size < 60) && (pkt->size > 38) && (strncasecmp((char *)"SET SESSION character_set_server",(char *)pkt->ptr+5,32)==0) ) { // issue #601
+	if (locked_on_hostgroup >= 0 && (strncasecmp((char *)"SET ",(char *)pkt->ptr+5,33)==4)) {
+		// this is a circuit breaker, we will send everything to the backend
+		//
+		// also note that in the current implementation we stop tracking variables:
+		// this becomes a problem if mysql-set_query_lock_on_hostgroup is
+		// disabled while a session is already locked
+		return false;
+	}
+	if ((pkt->size < 60) && (pkt->size > 38) && (strncasecmp((char *)"SET SESSION character_set_server",(char *)pkt->ptr+5,32)==0) ) { // issue #601
 		char *idx=NULL;
 		char *p=(char *)pkt->ptr+37;
 		idx=(char *)memchr(p,'=',pkt->size-37);
@@ -1199,7 +1207,7 @@ bool MySQL_Session::handler_special_queries(PtrSize_t *pkt) {
 			pkt->ptr=pkt_2.ptr;
 		}
 	}
-	if ((locked_on_hostgroup == -1) && (pkt->size < 60) && (pkt->size > 39) && (strncasecmp((char *)"SET SESSION character_set_results",(char *)pkt->ptr+5,33)==0) ) { // like the above
+	if ((pkt->size < 60) && (pkt->size > 39) && (strncasecmp((char *)"SET SESSION character_set_results",(char *)pkt->ptr+5,33)==0) ) { // like the above
 		char *idx=NULL;
 		char *p=(char *)pkt->ptr+38;
 		idx=(char *)memchr(p,'=',pkt->size-38);
@@ -1220,7 +1228,7 @@ bool MySQL_Session::handler_special_queries(PtrSize_t *pkt) {
 		}
 	}
 	if (
-		(locked_on_hostgroup == -1) && (pkt->size < 100) && (pkt->size > 15) && (strncasecmp((char *)"SET NAMES ",(char *)pkt->ptr+5,10)==0)
+		(pkt->size < 100) && (pkt->size > 15) && (strncasecmp((char *)"SET NAMES ",(char *)pkt->ptr+5,10)==0)
 		&&
 		(memchr((const void *)((char *)pkt->ptr+5),',',pkt->size-15)==NULL) // there is no comma
 	) {
@@ -2310,7 +2318,7 @@ bool MySQL_Session::handler_again___status_SETTING_GENERIC_VARIABLE(int *_rc, co
 					q=(char *)"SET %s=%s";
 				if (strncasecmp(var_value,(char *)"REPLACE",7)==0)
 					q=(char *)"SET %s=%s";
-				if (var_value[0] && var_value[0]=='(') {
+				if (var_value[0] && var_value[0]=='(') { // the value is a subquery
 					q=(char *)"SET %s=%s";
 				}
 			}
diff --git a/test/tap/tests/reg_test_3317-lock_hostgroup_special_queries-t.cpp b/test/tap/tests/reg_test_3317-lock_hostgroup_special_queries-t.cpp
index 285f3d707..71f041fe4 100644
--- a/test/tap/tests/reg_test_3317-lock_hostgroup_special_queries-t.cpp
+++ b/test/tap/tests/reg_test_3317-lock_hostgroup_special_queries-t.cpp
@@ -3,6 +3,8 @@
  * @brief This test verifies that after locking on a hostgroup, ProxySQL forwards
  *  several simple special queries in a proper way, forwarding them to the backend
  *  connection.
+ * Note: queries have hostgroup=0 to avoid getting lock on hostgroup 0 and
+ *       attempting to run queries on hostgroup 1
  */
 
 #include <cstring>
diff --git a/test/tap/tests/setparser_test.cpp b/test/tap/tests/setparser_test.cpp
index e16de87bd..990685290 100644
--- a/test/tap/tests/setparser_test.cpp
+++ b/test/tap/tests/setparser_test.cpp
@@ -3,6 +3,7 @@
  * @brief Test file for unit testing 'SetParser' type, responsible of parsing
  *   non-trivial 'SET' statements. This test is executed via the wrapper tap test
  *   'setparser_test-t'.
+ *   This file is an extension of ../../set_parser_test/setparsertest.cpp
  */
 
 #include "re2/re2.h"