Merge pull request #3249 from sysown/v2.0.16-collation

Do not search charset in mysql_real_connect if already set

deps/Makefile

@@ -172,6 +172,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
@@ -180,6 +181,7 @@ mariadb-client-library/mariadb_client/libmariadb/libmariadbclient.a: libssl/open
 	cd mariadb-client-library/mariadb_client && patch include/mariadb_com.h < ../mariadb_com.h.patch
 	cd mariadb-client-library/mariadb_client && patch libmariadb/ma_alloc.c < ../ma_alloc.c.patch
 	cd mariadb-client-library/mariadb_client && patch libmariadb/ma_charset.c < ../ma_charset.c.patch
+	cd mariadb-client-library/mariadb_client && patch libmariadb/ma_charset.c < ../ma_charset.c.german1.patch
 #	cd mariadb-client-library/mariadb_client && patch libmariadb/ma_pvio.c < ../ma_pvio.c.patch
 	cd mariadb-client-library/mariadb_client && patch unittest/libmariadb/basic-t.c < ../unittest_basic-t.c.patch
 	cd mariadb-client-library/mariadb_client && patch unittest/libmariadb/charset.c < ../unittest_charset.c.patch

deps/mariadb-client-library/ma_charset.c.german1.patch

@@ -0,0 +1,9 @@
+@@ -583,7 +583,7 @@ const MARIADB_CHARSET_INFO mariadb_compi
+   {  95, 1, "cp932", "cp932_japanese_ci", "", 932, "CP932", 1, 2, mysql_mbcharlen_cp932, check_mb_cp932},
+   {  97, 1, "eucjpms", "eucjpms_japanese_ci", "", 932, "EUC-JP-MS", 1, 3, mysql_mbcharlen_eucjpms, check_mb_eucjpms},
+   {   2, 1, "latin2", "latin2_czech_cs", "", 852, "LATIN2", 1, 1, NULL, NULL},
+-  {   5, 1, "latin1", "latin1_german_ci", "", 1252, "LATIN1", 1, 1, NULL, NULL},
++  {   5, 1, "latin1", "latin1_german1_ci", "", 1252, "LATIN1", 1, 1, NULL, NULL},
+   {  14, 1, "cp1251", "cp1251_bulgarian_ci", "", 1251, "CP1251", 1, 1, NULL, NULL},
+   {  15, 1, "latin1", "latin1_danish_ci", "", 1252, "LATIN1", 1, 1, NULL, NULL},
+   {  17, 1, "filename", "filename", "", 0, "", 1, 5, NULL, NULL},

deps/mariadb-client-library/mariadb_lib.c.collation.patch

@@ -0,0 +1,67 @@
+@@ -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)
+   {
+@@ -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:

lib/MySQL_HostGroups_Manager.cpp

@@ -674,6 +674,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;
@@ -3156,7 +3161,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);

lib/MySQL_Session.cpp

@@ -1063,6 +1063,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 : "" );

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");

lib/mysql_connection.cpp

@@ -696,7 +696,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;
@@ -767,6 +768,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);
 }
 

test/tap/tap/command_line.h

@@ -1,6 +1,8 @@
 #ifndef COMMAND_LINE_H
 #define COMMAND_LINE_H
 
+#include <string>
+
 class CommandLine {
 	public:
 	CommandLine();

test/tap/tests/Makefile

@@ -105,3 +105,6 @@ aurora: aurora.cpp $(TAP_LIBDIR)/libtap.a
 
 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)\"

test/tap/tests/test_ps_large_result-t.cpp

@@ -34,13 +34,16 @@ int select_config_file(MYSQL* mysql, std::string& resultset) {
 		fprintf(stderr, "error\n");
 	}
 
+	return 0;
 }
 
 int restore_admin(MYSQL* mysqladmin) {
 	MYSQL_QUERY(mysqladmin, "load mysql query rules from disk");
 	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,25 +122,22 @@ 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";
 	if (mysql_stmt_prepare(stmt1,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(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();