From d070395161c0ac3635ad61ea837b2391b7e13d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Thu, 14 Jan 2021 15:25:52 +0100 Subject: [PATCH 1/8] Do not search charset in mysql_real_connect if already set In mysql_init() charset is set to NULL . In mysql_read_connect() charset is not changed if already set This allows proxysql to change it outside the library --- deps/Makefile | 1 + .../mariadb_lib.c.collation.patch | 33 +++++++++++++++++++ lib/mysql_connection.cpp | 3 +- 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 deps/mariadb-client-library/mariadb_lib.c.collation.patch diff --git a/deps/Makefile b/deps/Makefile index d72e44599e..9cadc47b58 100644 --- a/deps/Makefile +++ b/deps/Makefile @@ -160,6 +160,7 @@ mariadb-client-library/mariadb_client/libmariadb/libmariadbclient.a: libssl/open # cd mariadb-client-library/mariadb_client && cmake . -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl # this is needed on MacOSX cd mariadb-client-library/mariadb_client && patch libmariadb/mariadb_stmt.c < ../mariadb_stmt.c.patch cd mariadb-client-library/mariadb_client && patch libmariadb/mariadb_lib.c < ../mariadb_lib.c.patch + cd mariadb-client-library/mariadb_client && patch libmariadb/mariadb_lib.c < ../mariadb_lib.c.collation.patch # make sure this path is applied after mariadb_lib.c.patch # cd mariadb-client-library/mariadb_client && patch libmariadb/net.c < ../net.c.patch cd mariadb-client-library/mariadb_client && patch libmariadb/mariadb_async.c < ../mariadb_async.c.patch cd mariadb-client-library/mariadb_client && patch libmariadb/ma_password.c < ../ma_password.c.patch diff --git a/deps/mariadb-client-library/mariadb_lib.c.collation.patch b/deps/mariadb-client-library/mariadb_lib.c.collation.patch new file mode 100644 index 0000000000..ecb41ff890 --- /dev/null +++ b/deps/mariadb-client-library/mariadb_lib.c.collation.patch @@ -0,0 +1,33 @@ +@@ -1017,7 +1017,10 @@ mysql_init(MYSQL *mysql) + goto error; + mysql->options.report_data_truncation= 1; + mysql->options.connect_timeout=CONNECT_TIMEOUT; +- mysql->charset= mysql_find_charset_name(MARIADB_DEFAULT_CHARSET); ++ // in proxysql we set mysql->charset to NULL during mysql_init() ++ // proxysql will explicitly set it a value if needed ++ mysql->charset = NULL; ++ //mysql->charset= mysql_find_charset_name(MARIADB_DEFAULT_CHARSET); + mysql->methods= &MARIADB_DEFAULT_METHODS; + strcpy(mysql->net.sqlstate, "00000"); + mysql->net.last_error[0]= mysql->net.last_errno= 0; +@@ -1497,11 +1500,15 @@ MYSQL *mthd_my_real_connect(MYSQL *mysql + } + } + +- /* Set character set */ +- if (mysql->options.charset_name) +- mysql->charset= mysql_find_charset_name(mysql->options.charset_name); +- else +- mysql->charset=mysql_find_charset_name(MARIADB_DEFAULT_CHARSET); ++ if (!mysql->charset) { // in proxysql we do not set charset during mysql_init ++ /* Set character set */ ++ if (mysql->options.charset_name) ++ mysql->charset= mysql_find_charset_name(mysql->options.charset_name); ++ else ++ mysql->charset=mysql_find_charset_name(MARIADB_DEFAULT_CHARSET); ++ } else { ++ // proxysql has explicitly set charset ++ } + + if (!mysql->charset) + { diff --git a/lib/mysql_connection.cpp b/lib/mysql_connection.cpp index 7046ccec5d..0a6b44f873 100644 --- a/lib/mysql_connection.cpp +++ b/lib/mysql_connection.cpp @@ -574,7 +574,8 @@ void MySQL_Connection::connect_start() { mysql_variables.server_set_value(myds->sess, SQL_CHARACTER_SET_CONNECTION, ss.str().c_str()); mysql_variables.server_set_value(myds->sess, SQL_COLLATION_CONNECTION, ss.str().c_str()); } - mysql_options(mysql, MYSQL_SET_CHARSET_NAME, c->csname); + //mysql_options(mysql, MYSQL_SET_CHARSET_NAME, c->csname); + mysql->charset = c; unsigned long client_flags = 0; //if (mysql_thread___client_found_rows) // client_flags += CLIENT_FOUND_ROWS; From 236e7a0fd836c7c5444352f4a420c26a2ff4e635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Thu, 14 Jan 2021 22:59:50 +0100 Subject: [PATCH 2/8] Do ot use charset_name if not available in SQL3_Free_Connections() This fix a crashing bug --- lib/MySQL_HostGroups_Manager.cpp | 4 +++- lib/MySQL_Session.cpp | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/MySQL_HostGroups_Manager.cpp b/lib/MySQL_HostGroups_Manager.cpp index ce7c59c46f..0aee6acbad 100644 --- a/lib/MySQL_HostGroups_Manager.cpp +++ b/lib/MySQL_HostGroups_Manager.cpp @@ -3579,7 +3579,9 @@ SQLite3_result * MySQL_HostGroups_Manager::SQL3_Free_Connections() { j["thread_id"] = _my->thread_id; j["server_status"] = _my->server_status; j["charset"] = _my->charset->nr; - j["options"]["charset_name"] = _my->options.charset_name; + j["charset_name"] = _my->charset->csname; + + j["options"]["charset_name"] = ( _my->options.charset_name ? _my->options.charset_name : "" ); j["options"]["use_ssl"] = _my->options.use_ssl; j["client_flag"]["client_found_rows"] = (_my->client_flag & CLIENT_FOUND_ROWS ? 1 : 0); j["client_flag"]["client_multi_statements"] = (_my->client_flag & CLIENT_MULTI_STATEMENTS ? 1 : 0); diff --git a/lib/MySQL_Session.cpp b/lib/MySQL_Session.cpp index e90fe322c1..09a23d7c74 100644 --- a/lib/MySQL_Session.cpp +++ b/lib/MySQL_Session.cpp @@ -1065,6 +1065,7 @@ void MySQL_Session::generate_proxysql_internal_session_json(json &j) { j["backends"][i]["conn"]["mysql"]["thread_id"] = _my->thread_id; j["backends"][i]["conn"]["mysql"]["server_status"] = _my->server_status; j["backends"][i]["conn"]["mysql"]["charset"] = _my->charset->nr; + j["backends"][i]["conn"]["mysql"]["charset_name"] = _my->charset->csname; //j["backends"][i]["conn"]["mysql"][""] = _my->; //j["backends"][i]["conn"]["mysql"][""] = _my->; j["backends"][i]["conn"]["mysql"]["options"]["charset_name"] = ( _my->options.charset_name ? _my->options.charset_name : "" ); From ee46096e1826854444ac0e91b017cf76edd76f38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Fri, 15 Jan 2021 16:24:52 +0000 Subject: [PATCH 3/8] Fixed undefined behavior in 'test_ps_large_result-t.cpp-t' due non-void non returning functions --- test/tap/tests/test_ps_large_result-t.cpp | 50 +++++++++-------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/test/tap/tests/test_ps_large_result-t.cpp b/test/tap/tests/test_ps_large_result-t.cpp index c1e41f0737..9cecb78c51 100644 --- a/test/tap/tests/test_ps_large_result-t.cpp +++ b/test/tap/tests/test_ps_large_result-t.cpp @@ -34,6 +34,7 @@ int select_config_file(MYSQL* mysql, std::string& resultset) { fprintf(stderr, "error\n"); } + return 0; } int restore_admin(MYSQL* mysqladmin) { @@ -41,6 +42,8 @@ int restore_admin(MYSQL* mysqladmin) { MYSQL_QUERY(mysqladmin, "load mysql query rules to runtime"); MYSQL_QUERY(mysqladmin, "load mysql servers from disk"); MYSQL_QUERY(mysqladmin, "load mysql servers to runtime"); + + return 0; } int main(int argc, char** argv) { @@ -119,8 +122,7 @@ int main(int argc, char** argv) { if (!stmt1) { ok(false, " mysql_stmt_init(), out of memory\n"); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } std::string query = "SELECT id FROM test.sbtest1 LIMIT 100"; @@ -128,16 +130,14 @@ int main(int argc, char** argv) { fprintf(stderr, "Query error %s\n", mysql_error(mysql)); mysql_close(mysql); mysql_library_end(); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } if (mysql_stmt_execute(stmt1)) { fprintf(stderr, " mysql_stmt_execute(), failed\n"); ok(false, " %s\n", mysql_stmt_error(stmt1)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } ok(true, "100 rows result stored"); @@ -159,16 +159,14 @@ int main(int argc, char** argv) { { fprintf(stderr, " mysql_stmt_bind_result() failed\n"); ok(false, " %s\n", mysql_stmt_error(stmt1)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } if (mysql_stmt_store_result(stmt1)) { fprintf(stderr, " mysql_stmt_store_result() failed\n"); ok(false, " %s\n", mysql_stmt_error(stmt1)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } while (!mysql_stmt_fetch(stmt1)) @@ -180,31 +178,27 @@ int main(int argc, char** argv) { { fprintf(stderr, " failed while closing the statement\n"); ok(false, " %s\n", mysql_error(mysql)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } MYSQL_STMT *stmt2 = mysql_stmt_init(mysql); if (!stmt2) { ok(false, " mysql_stmt_init(), out of memory\n"); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } query = "SELECT t1.id id1, t1.k k1, t1.c c1, t1.pad pad1, t2.id id2, t2.k k2, t2.c c2, t2.pad pad2 FROM test.sbtest1 t1 JOIN test.sbtest1 t2 LIMIT 10000000"; if (mysql_stmt_prepare(stmt2,query.c_str(), query.size())) { fprintf(stderr, "Query error %s\n", mysql_error(mysql)); mysql_close(mysql); mysql_library_end(); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } if (mysql_stmt_execute(stmt2)) { fprintf(stderr, " mysql_stmt_execute(), failed\n"); ok(false, " %s\n", mysql_stmt_error(stmt2)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } ok(true, "4GB resultset stored"); @@ -281,16 +275,14 @@ int main(int argc, char** argv) { { fprintf(stderr, " mysql_stmt_bind_result() failed\n"); ok(false, " %s\n", mysql_stmt_error(stmt2)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } if (mysql_stmt_store_result(stmt2)) { fprintf(stderr, " mysql_stmt_store_result() failed\n"); ok(false, " %s\n", mysql_stmt_error(stmt2)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } while (!mysql_stmt_fetch(stmt2)) @@ -305,8 +297,7 @@ int main(int argc, char** argv) { { fprintf(stderr, " failed while closing the statement\n"); ok(false, " %s\n", mysql_error(mysql)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } @@ -315,8 +306,7 @@ int main(int argc, char** argv) { if (!stmt3) { ok(false, " mysql_stmt_init(), out of memory\n"); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } /* Test case #3. */ @@ -326,8 +316,7 @@ int main(int argc, char** argv) { fprintf(stderr, "Query error %s\n", mysql_error(mysql)); mysql_close(mysql); mysql_library_end(); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } /* Execute the SELECT query */ @@ -414,14 +403,13 @@ int main(int argc, char** argv) { { fprintf(stderr, " failed while closing the statement\n"); ok(false, " %s\n", mysql_error(mysql)); - restore_admin(mysqladmin); - return exit_status(); + return restore_admin(mysqladmin); } if (str_data32) free(str_data32); - restore_admin(mysqladmin); + return restore_admin(mysqladmin); mysql_close(mysql); mysql_library_end(); From c3e774428b7bd87b8c9cd924ef6673ae40bb093a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Sun, 17 Jan 2021 20:38:02 +0100 Subject: [PATCH 4/8] Do not search charset in mysql_change_user if already set This is an extension of commit 1ecb00f19fd55e6a97d5e3ea3b18039fa38d4af2 about mysql_real_connect It also changes mysql_optionsv() for MYSQL_SET_CHARSET_NAME --- .../mariadb_lib.c.collation.patch | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/deps/mariadb-client-library/mariadb_lib.c.collation.patch b/deps/mariadb-client-library/mariadb_lib.c.collation.patch index ecb41ff890..9067537d81 100644 --- a/deps/mariadb-client-library/mariadb_lib.c.collation.patch +++ b/deps/mariadb-client-library/mariadb_lib.c.collation.patch @@ -31,3 +31,37 @@ if (!mysql->charset) { +@@ -1759,10 +1766,16 @@ my_bool STDCALL mysql_change_user(MYSQL + *s_db= mysql->db; + int rc; + +- if (mysql->options.charset_name) +- mysql->charset= mysql_find_charset_name(mysql->options.charset_name); +- else +- mysql->charset=mysql_find_charset_name(MARIADB_DEFAULT_CHARSET); ++ // in proxysql we set charset directly, ++ // therefore this code should never be called in proxysql. ++ // we keep the code because compatibility (for example, an app using mysql_change_user) ++ // we also change mysql_optionsv() for MYSQL_SET_CHARSET_NAME ++ if (!mysql->charset) { ++ if (mysql->options.charset_name) ++ mysql->charset= mysql_find_charset_name(mysql->options.charset_name); ++ else ++ mysql->charset=mysql_find_charset_name(MARIADB_DEFAULT_CHARSET); ++ } + + mysql->user= strdup(user ? user : ""); + mysql->passwd= strdup(passwd ? passwd : ""); +@@ -2767,6 +2780,12 @@ mysql_optionsv(MYSQL *mysql,enum mysql_o + OPT_SET_VALUE_STR(&mysql->options, charset_dir, arg1); + break; + case MYSQL_SET_CHARSET_NAME: ++ { ++ // this is for applications other than proxysql. ++ // This because proxysql doesn't use mysql_options() with MYSQL_SET_CHARSET_NAME , ++ // but instead set mysql->charset directly ++ mysql->charset = NULL; ++ } + OPT_SET_VALUE_STR(&mysql->options, charset_name, arg1); + break; + case MYSQL_OPT_RECONNECT: From 1618cf648b485510c8e2fb8793ba057f482e58be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= Date: Sun, 17 Jan 2021 23:32:50 +0100 Subject: [PATCH 5/8] When calling mysql_change_user set collation to default for the given charset if collation ID is greater or equal than 255 (utf8mb4_0900_ai_ci) We call mysql_options with MYSQL_SET_CHARSET_NAME if collation ID >= 255 . This to solve the problem of MySQL server returning an Access denied if the backend doesn't support a collation during mysql_change_user --- lib/MySQL_HostGroups_Manager.cpp | 5 +++++ lib/mysql_connection.cpp | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/lib/MySQL_HostGroups_Manager.cpp b/lib/MySQL_HostGroups_Manager.cpp index 0aee6acbad..e3540cb193 100644 --- a/lib/MySQL_HostGroups_Manager.cpp +++ b/lib/MySQL_HostGroups_Manager.cpp @@ -686,6 +686,11 @@ static void * HGCU_thread_run() { } } //async_exit_status = mysql_change_user_start(&ret_bool,mysql,_ui->username, auth_password, _ui->schemaname); + // we first reset the charset to a default one. + // this to solve the problem described here: + // https://github.com/sysown/proxysql/pull/3249#issuecomment-761887970 + if (myconn->mysql->charset->nr >= 255) + mysql_options(myconn->mysql, MYSQL_SET_CHARSET_NAME, myconn->mysql->charset->csname); statuses[i]=mysql_change_user_start(&ret[i], myconn->mysql, myconn->userinfo->username, auth_password, myconn->userinfo->schemaname); if (myconn->mysql->net.pvio==NULL || myconn->mysql->net.fd==0 || myconn->mysql->net.buff==NULL) { statuses[i]=0; ret[i]=1; diff --git a/lib/mysql_connection.cpp b/lib/mysql_connection.cpp index 0a6b44f873..5ab2ca4f49 100644 --- a/lib/mysql_connection.cpp +++ b/lib/mysql_connection.cpp @@ -646,6 +646,11 @@ void MySQL_Connection::change_user_start() { auth_password=userinfo->password; } } + // we first reset the charset to a default one. + // this to solve the problem described here: + // https://github.com/sysown/proxysql/pull/3249#issuecomment-761887970 + if (mysql->charset->nr >= 255) + mysql_options(mysql, MYSQL_SET_CHARSET_NAME, mysql->charset->csname); async_exit_status = mysql_change_user_start(&ret_bool,mysql,_ui->username, auth_password, _ui->schemaname); } From cdd987845591ec01c5d2eb81252be167653b9961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Tue, 19 Jan 2021 13:31:54 +0000 Subject: [PATCH 6/8] Added missing required include in 'command_line.h' --- test/tap/tap/command_line.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/tap/tap/command_line.h b/test/tap/tap/command_line.h index c4857c2992..5d008f1f27 100644 --- a/test/tap/tap/command_line.h +++ b/test/tap/tap/command_line.h @@ -1,6 +1,8 @@ #ifndef COMMAND_LINE_H #define COMMAND_LINE_H +#include + class CommandLine { public: CommandLine(); From c44b169d4457a56f4b9ec2dc3c25605f7a305393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Tue, 19 Jan 2021 13:32:11 +0000 Subject: [PATCH 7/8] Added new test 'test_set_collation-t' to verify that ProxySQL is properly setting collation specified by the client --- test/tap/tests/Makefile | 6 + test/tap/tests/test_set_collation-t.cpp | 159 ++++++++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 test/tap/tests/test_set_collation-t.cpp diff --git a/test/tap/tests/Makefile b/test/tap/tests/Makefile index 15e4c5cced..9305d3bd79 100644 --- a/test/tap/tests/Makefile +++ b/test/tap/tests/Makefile @@ -109,3 +109,9 @@ test_gtid_forwarding-t: test_gtid_forwarding-t.cpp $(TAP_LIBDIR)/libtap.a test_admin_prometheus_metrics_dump-t: test_admin_prometheus_metrics_dump-t.cpp $(TAP_LIBDIR)/libtap.a g++ test_admin_prometheus_metrics_dump-t.cpp $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 $(MYLIBS) -ltap -Wl,--no-as-needed -ldl -lpthread -o test_admin_prometheus_metrics_dump-t -DGITVERSION=\"$(GIT_VERSION)\" + +create_connection_annotation: test_connection_annotation-t.cpp + g++ -DTEST_AURORA -DDEBUG test_connection_annotation-t.cpp $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 $(OBJ) $(MYLIBS) -ltap -ldl $(STATIC_LIBS) -o test_connection_annotation-t -DGITVERSION=\"$(GIT_VERSION)\" + +test_set_collation-t: test_set_collation-t.cpp $(TAP_LIBDIR)/libtap.a + g++ test_set_collation-t.cpp $(INCLUDEDIRS) $(LDIRS) $(OPT) -std=c++11 $(MYLIBS) -ltap -Wl,--no-as-needed -ldl -lpthread -o test_set_collation-t -DGITVERSION=\"$(GIT_VERSION)\" diff --git a/test/tap/tests/test_set_collation-t.cpp b/test/tap/tests/test_set_collation-t.cpp new file mode 100644 index 0000000000..636967f0b1 --- /dev/null +++ b/test/tap/tests/test_set_collation-t.cpp @@ -0,0 +1,159 @@ +/** + * @file test_set_collation-t.cpp + * @brief This test verifies that ProxySQL is properly setting the specified collation by the + * client during connection phase. + */ + +#include +#include +#include + +#include "tap.h" +#include "command_line.h" +#include "utils.h" + +/** + * NOTE: This is a duplicate of 'proxysql_find_charset_collate' in 'MySQL_Variables.h'. Including + * 'MySQL_Variables' is not a easy task due to its interdependeces with other ProxySQL modules. + */ +MARIADB_CHARSET_INFO * proxysql_find_charset_collate(const char *collatename) { + MARIADB_CHARSET_INFO *c = (MARIADB_CHARSET_INFO *)mariadb_compiled_charsets; + do { + if (!strcasecmp(c->name, collatename)) { + return c; + } + ++c; + } while (c[0].nr != 0); + return NULL; +} + +/** + * @brief Creates a different MYSQL connection for each supplied collation. Logs in case of a + * failure creating a connection. + * + * @param cl The command line parameters required for creating the connection. + * @param collations The collations with which to initialize the MYSQL connections. + * @param conns Output parameter which will contain the initialized connections in case of success. + * + * @return Returns '0' in case of success, '-1' otherwise. + */ +int create_proxysql_connections(const CommandLine& cl, const std::vector& collations, std::vector& conns) { + std::vector _conns {}; + + for (const auto& collation : collations) { + MYSQL* mysql = mysql_init(NULL); + const MARIADB_CHARSET_INFO* charset = proxysql_find_charset_collate(collation.c_str()); + mysql->charset = charset; + + if (!mysql_real_connect(mysql, cl.host, cl.username, cl.password, NULL, cl.port, NULL, 0)) { + fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(mysql)); + return -1; + } + + _conns.push_back(mysql); + } + + // return the created connections + conns = _conns; + + return 0; +} + +int main(int argc, char** argv) { + CommandLine cl; + + if (cl.getEnv()) { + diag("Failed to get the required environmental variables."); + return -1; + } + + int iterations = 10; + std::vector conns {}; + std::vector collations { "latin1_spanish_ci", "latin1_german2_ci", "latin1_danish_ci", "latin1_general_ci", "latin1_bin" }; + int conns_res = create_proxysql_connections(cl, collations, conns); + + ok(conns_res == 0, "Successfully create all connections with different collations"); + if (conns_res != 0) { + return exit_status(); + } + + /** + * Force ProxySQL to create two new backend connections and simultaneously check that the + * 'character_set%' and 'collation_connection' for them are correct. + */ + for (int i = 0; i < 2; i++) { + MYSQL* mysql = conns[i]; + std::string collation = collations[i]; + MYSQL_RES* proxy_res = nullptr; + + MYSQL_QUERY( + mysql, + "/*+ ;create_new_connection=1 */ SELECT lower(variable_name), variable_value FROM performance_schema.session_variables WHERE" + " Variable_name IN ('character_set_client', 'character_set_connection', 'character_set_results', 'collation_connection')" + ); + proxy_res = mysql_store_result(mysql); + + MYSQL_ROW row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_client' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_connection' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_results' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], collation.c_str()) == 0, "'collation_connection' matches (expected: '%s') != (actual: '%s')", collation.c_str(), row[1]); + } + + /** + * Do multiple iterations over the created connections checking that the 'character_set_%' and 'collation' variables + * remain properly set in the client and server side. + */ + for (int i = 0; i < conns.size(); i++) { + MYSQL* mysql = conns[i]; + std::string collation = collations[i]; + MYSQL_RES* proxy_res = nullptr; + + MYSQL_QUERY( + mysql, + "SHOW VARIABLES WHERE Variable_name IN ('character_set_client', 'character_set_connection', 'character_set_results', 'collation_connection')" + ); + proxy_res = mysql_store_result(mysql); + + MYSQL_ROW row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_client' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_connection' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_results' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], collation.c_str()) == 0, "'collation_connection' matches (expected: '%s') != (actual: '%s')", collation.c_str(), row[1]); + + for (int j = 0; j < iterations; j++) { + MYSQL_QUERY( + mysql, + "SELECT lower(variable_name), variable_value FROM performance_schema.session_variables WHERE" + " Variable_name IN ('character_set_client', 'character_set_connection', 'character_set_results', 'collation_connection')" + ); + proxy_res = mysql_store_result(mysql); + + MYSQL_ROW row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_client' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_connection' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], "latin1") == 0, "'character_set_results' matches (expected: 'latin1') != (actual: '%s')", row[1]); + + row = mysql_fetch_row(proxy_res); + ok(strcmp(row[1], collation.c_str()) == 0, "'collation_connection' matches (expected: '%s') != (actual: '%s')", collation.c_str(), row[1]); + } + } + + return exit_status(); +} From 776f1ff516e52be13c0966a3f7cf6645d843d06f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Jaramago=20Fern=C3=A1ndez?= Date: Tue, 19 Jan 2021 13:32:21 +0000 Subject: [PATCH 8/8] Added more detailed documentation to 'MySQL_Variables::client_set_value' --- lib/MySQL_Variables.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/lib/MySQL_Variables.cpp b/lib/MySQL_Variables.cpp index 02ed6d3b48..2c56553b1c 100644 --- a/lib/MySQL_Variables.cpp +++ b/lib/MySQL_Variables.cpp @@ -86,6 +86,38 @@ void MySQL_Variables::server_set_hash_and_value(MySQL_Session* session, int idx, } +/** + * @brief Set the supplied value for the session variable specified by the supplied + * index into the supplied client session. + * + * @details There are two session variables which require special handling: + * - 'SET NAMES' + * - 'SET CHARACTER SET' + * + * For the second case 'SET CHARACTER SET' we forget about the values for: + * - 'SQL_CHARACTER_SET_CONNECTION' + * - 'SQL_COLLATION_CONNECTION' + * + * This is done because 'character_set_database' is not known when the set happens and + * because we work under the assumption that if a client request 'SET CHARACTER SET' + * doesn't require a specific 'collation_connection' and 'character_set_connection". + * Furthermore, 'character_set_database' is deprecated since MySQL 5.7 and will only + * be usable as an immutable session variable in the future. For reference see: + * + * - 'https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_character_set_database + * + * Due to this, *it's expected behavior* to see that a connection that sets 'SET CHARACTER SET' + * has a variant 'collation_connection' and 'character_set_connection' depending on the + * backend connection that is retrieved from the connection pool. If the 'collation_connection' + * and 'character_set_connection' variables are relevant and should never change, + * 'SET NAMES' should be used. + * + * @param session The client session which variable value is going to be modified. + * @param idx The index of the session variable to modify. + * @param value The session variable value to be set. + * + * @return 'true' in case of success, 'false' otherwise. + */ bool MySQL_Variables::client_set_value(MySQL_Session* session, int idx, const std::string& value) { if (!session || !session->client_myds || !session->client_myds->myconn) { proxy_warning("Session validation failed\n");