MDEV-14454 Binary protocol returns wrong collation ID for SP OUT parameters

Item_param::set_value() did not set Item::collation and
Item_param::str_value_ptr.str_charset properly. So both
metadata and data for OUT parameters were sent in a wrong
way to the client.

This patch removes the old implementation of Item_param::set_value()
and rewrites it using Type_handler::Item_param_set_from_value(),
so now setting IN and OUT parameters share the a lot of code.

1. Item_param::set_str() now:
  - accepts two additional parameters fromcs, tocs
  - sets str_value_ptr, to make sure it's always in sync with str_value,
    even without Item_param::convert_str_value()
  - does collation.set(tocs, DERIVATION_COERCIBLE),
    to make sure that DTCollation is valid even without
    Item_param::convert_str_value()

2. Item_param::set_value(), which is used to set OUT parameters,
   now reuses Type_handler::Item_param_set_from_value().

3. Cleanup: moving Item_param::str_value_ptr to private,
   as it's not needed outside.

4. Cleanup: adding a new virtual method
   Settable_routine_parameter::get_item_param()
   and using it a few new DBUG_ASSERTs, where
   Item_param cannot appear.

After this change:
1. Assigning of IN parameters works as before:
a. Item_param::set_str() is called and sets the value as a binary string
b. The original value is sent to the query used for binary/general logging
c. Item_param::convert_str_value() converts the value from the client
   character set to the connection character set

2. Assigning of OUT parameters works in the new way:
a. Item_param::set_str() and sets the value
   using the source Item's collation, so both Item::collation
   and Item_param::str_value_ptr.str_charset are properly set.
b. Protocol_binary::send_out_parameters() sends the
   value to the client correctly:
   - Protocol::send_result_set_metadata() uses Item::collation.collation
     (which is now properly set), to detect if conversion is needed,
     and sends a correct collation ID.
   - Protocol::send_result_set_row() calls Type_handler::Item_send_str(),
     which uses Item_param::str_value_ptr.str_charset
     (which is now properly set) to actually perform the conversion.

2 files changed, 89 insertions, 0 deletions

diff --git a/tests/mysql_client_fw.c b/tests/mysql_client_fw.c
index bf06e2b502b..622183d527a 100644
--- a/tests/mysql_client_fw.c
+++ b/tests/mysql_client_fw.c
@@ -1177,6 +1177,15 @@ static my_bool thread_query(const char *query)
 }
 
 
+static int mysql_query_or_error(MYSQL *mysql, const char *query)
+{
+  int rc= mysql_query(mysql, query);
+  if (rc)
+    fprintf(stderr, "ERROR %d: %s", mysql_errno(mysql), mysql_error(mysql));
+  return rc;
+}
+
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c
index ae5c70db1a8..0b67a9414fd 100644
--- a/tests/mysql_client_test.c
+++ b/tests/mysql_client_test.c
@@ -19858,6 +19858,85 @@ static void test_mdev14013_1()
 }
 
 
+static void test_mdev14454_internal(const char *init,
+                                    unsigned int csid,
+                                    const char *value)
+{
+  MYSQL_STMT *stmt;
+  MYSQL_BIND bind;
+  const char *stmtstr= "CALL P1(?)";
+  char res[20];
+  int rc;
+
+  if ((rc= mysql_query_or_error(mysql, init)) ||
+      (rc= mysql_query_or_error(mysql, "DROP PROCEDURE IF EXISTS p1")) ||
+      (rc= mysql_query_or_error(mysql,
+                               "CREATE PROCEDURE p1"
+                               "("
+                               "  OUT param1 TEXT CHARACTER SET utf8"
+                               ")"
+                               "BEGIN "
+                               "  SET param1 = _latin1'test\xFF'; "
+                               "END")))
+    DIE("Initiation failed");
+
+  stmt= mysql_stmt_init(mysql);
+  rc= mysql_stmt_prepare(stmt, stmtstr, strlen(stmtstr));
+  DIE_UNLESS(rc == 0);
+  DIE_UNLESS(mysql_stmt_param_count(stmt) == 1);
+
+  bind.buffer_type= MYSQL_TYPE_NULL;
+  rc= mysql_stmt_bind_param(stmt, &bind);
+  DIE_UNLESS(rc == 0);
+
+  rc= mysql_stmt_execute(stmt);
+  DIE_UNLESS(rc == 0);
+
+  memset(res, 0, sizeof(res));
+  memset(&bind, 0, sizeof(bind));
+  bind.buffer_type= MYSQL_TYPE_STRING;
+  bind.buffer_length= sizeof(res);
+  bind.buffer= res;
+
+  do {
+    if (mysql->server_status & SERVER_PS_OUT_PARAMS)
+    {
+      MYSQL_FIELD *field;
+      printf("\nOUT param result set:\n");
+      DIE_UNLESS(mysql_stmt_field_count(stmt) == 1);
+      field= &stmt->fields[0];
+      printf("Field: %s\n", field->name);
+      printf("Type: %d\n", field->type);
+      printf("Collation: %d\n", field->charsetnr);
+      printf("Length: %lu\n", field->length);
+      DIE_UNLESS(stmt->fields[0].charsetnr == csid);
+
+      rc= mysql_stmt_bind_result(stmt, &bind);
+      DIE_UNLESS(rc == 0);
+      rc= mysql_stmt_fetch(stmt);
+      DIE_UNLESS(rc == 0);
+      printf("Value: %s\n", res);
+      DIE_UNLESS(strcmp(res, value) == 0);
+    }
+    else if (mysql_stmt_field_count(stmt))
+    {
+      printf("sp result set\n");
+    }
+  } while (mysql_stmt_next_result(stmt) == 0);
+
+  mysql_stmt_close(stmt);
+  DIE_UNLESS(mysql_query_or_error(mysql, "DROP PROCEDURE p1") == 0);
+}
+
+
+static void test_mdev14454()
+{
+  myheader("test_mdev14454");
+  test_mdev14454_internal("SET NAMES latin1", 8, "test\xFF");
+  test_mdev14454_internal("SET NAMES utf8", 33, "test\xC3\xBF");
+}
+
+
 static struct my_tests_st my_tests[]= {
   { "disable_query_logs", disable_query_logs },
   { "test_view_sp_list_fields", test_view_sp_list_fields },
@@ -20139,6 +20218,7 @@ static struct my_tests_st my_tests[]= {
   { "test_mdev12579", test_mdev12579 },
   { "test_mdev14013", test_mdev14013 },
   { "test_mdev14013_1", test_mdev14013_1 },
+  { "test_mdev14454", test_mdev14454 },
   { 0, 0 }
 };