← Back to team overview

maria-developers team mailing list archive

Re: [Commits] Rev 3197: MDEV-4326 fix.

 

Hi, Sanja!

On Apr 09, sanja@xxxxxxxxxxxx wrote:
> revno: 3197
> revision-id: sanja@xxxxxxxxxxxx-20130409065851-bb241d1scc8lxo3p
> parent: sergii@xxxxxxxxx-20130406192912-jtwuhe78zijfqfbx
> committer: sanja@xxxxxxxxxxxx
> branch nick: work-maria-5.1-MDEV-4326
> timestamp: Tue 2013-04-09 09:58:51 +0300
> message:
>   MDEV-4326 fix.
>   
>   Removed "optimization" which caused preoblems on second execution of
>   PS with string parameter in LIMIT clause.
>   
> === modified file 'sql/sql_prepare.cc'
> --- a/sql/sql_prepare.cc	2012-11-06 09:52:55 +0000
> +++ b/sql/sql_prepare.cc	2013-04-09 06:58:51 +0000
> @@ -787,7 +787,7 @@ static bool insert_params_with_log(Prepa
>          if (param->state == Item_param::NO_VALUE)
>            DBUG_RETURN(1);
>  
> -        if (param->limit_clause_param && param->item_type != Item::INT_ITEM)
> +        if (param->limit_clause_param)

This looks wrong. Why do you need to convert INT_ITEM to INT_ITEM ?

>          {
>            param->set_int(param->val_int(), MY_INT64_NUM_DECIMAL_DIGITS);
>            param->item_type= Item::INT_ITEM;
> 
> === modified file 'tests/mysql_client_test.c'
> --- a/tests/mysql_client_test.c	2013-04-06 19:29:12 +0000
> +++ b/tests/mysql_client_test.c	2013-04-09 06:58:51 +0000
> @@ -16750,7 +16750,11 @@ static void test_bug43560(void)
>      fprintf(stdout, "Skipping test_bug43560: server not DEBUG version\n");
>      DBUG_VOID_RETURN;
>    }
> -
> +  if (opt_unix_socket)
> +  {
> +    fprintf(stdout, "Skipping test_bug43560: connected via UNIX socket\n");
> +    DBUG_VOID_RETURN;
> +  }

Why is that? There's nothing in the bug depends on how the client is
connected.

>    /*
>      Set up a separate connection for this test to avoid messing up the
>      general MYSQL object used in other subtests. Use TCP protocol to avoid
> @@ -17541,6 +17545,109 @@ static void test_bug13001491()
>    myquery(rc);
>  }
>  
> +static void test_mdev4326()
> +{
> +  MYSQL_STMT   *stmt;
> +  MYSQL_BIND    bind;
> +  char query[]= "SELECT * FROM mysql.user LIMIT ?";
> +  char str_data[]= "1";
> +  unsigned long length= 0;
> +  int int_data= 1;
> +  int rc, count;
> +  my_bool is_null= 0;
> +  my_bool error= 0;
> +  myheader("test_mdev4326");
> +
> +  rc= mysql_change_user(mysql, opt_user, opt_password, "mysql");
> +  myquery(rc);

Sanja, I asked to reduce the test case and to remove whatever is not
relevant to the bug. Why do you need to change user here? It's not
needed for the bug to show up.

> +
> +  rc= mysql_query(mysql, "SET GLOBAL general_log = 1");
> +  myquery(rc);
> +
> +  stmt= mysql_stmt_init(mysql);
> +  check_stmt(stmt);
> +
> +  /* Numeric parameter test */
> +
> +  rc= mysql_stmt_prepare(stmt, query, strlen(query));
> +  check_execute(stmt, rc);
> +  check_stmt(stmt);
> +  verify_param_count(stmt, 1);
> +
> +  memset((char *)&bind, 0, sizeof(bind));
> +  bind.buffer_type= MYSQL_TYPE_LONG;
> +  bind.buffer= (char *)&int_data;
> +  bind.is_null= &is_null;
> +  bind.length= &length;
> +  bind.error= &error;

Why do you execute the statement with MYSQL_TYPE_LONG parameters?
It's not needed for the bug to show up.

> +
> +  rc= mysql_stmt_bind_param(stmt, &bind);
> +  check_execute(stmt, rc);
> +  rc= mysql_stmt_execute(stmt);
> +  check_execute(stmt, rc);
> +  count= 0;
> +  while (!(rc= mysql_stmt_fetch(stmt)))
> +    count++;
> +  DIE_UNLESS(count == 1);
> +  rc= mysql_stmt_execute(stmt);
> +  check_execute(stmt, rc);
> +  count= 0;
> +  while (!(rc= mysql_stmt_fetch(stmt)))
> +    count++;
> +  DIE_UNLESS(count == 1);
> +  int_data= 0;
> +  rc= mysql_stmt_execute(stmt);
> +  check_execute(stmt, rc);
> +  count= 0;
> +  while (!(rc= mysql_stmt_fetch(stmt)))
> +    count++;
> +  DIE_UNLESS(count == 0);
> +  rc= mysql_stmt_close(stmt);
> +  check_execute(stmt, rc);
> +
> +  /* String parameter test */
> +
> +  stmt= mysql_stmt_init(mysql);
> +  rc= mysql_stmt_prepare(stmt, query, strlen(query));
> +  check_execute(stmt, rc);
> +  check_stmt(stmt);
> +  verify_param_count(stmt, 1);
> +
> +  memset((char *)&bind, 0, sizeof(bind));
> +  bind.buffer_type= MYSQL_TYPE_STRING;
> +  bind.buffer= (char *)str_data;
> +  length= bind.buffer_length= sizeof(str_data);
> +  bind.is_null= &is_null;
> +  bind.length= &length;
> +  bind.error= &error;
> +
> +  rc= mysql_stmt_bind_param(stmt, &bind);
> +  check_execute(stmt, rc);
> +  rc= mysql_stmt_execute(stmt);
> +  check_execute(stmt, rc);
> +  count= 0;
> +  while (!(rc= mysql_stmt_fetch(stmt)))
> +    count++;
> +  DIE_UNLESS(count == 1);
> +  rc= mysql_stmt_execute(stmt);
> +  check_execute(stmt, rc);
> +  count= 0;
> +  while (!(rc= mysql_stmt_fetch(stmt)))
> +    count++;
> +  DIE_UNLESS(count == 1);
> +  str_data[0]= '0';
> +  rc= mysql_stmt_execute(stmt);
> +  check_execute(stmt, rc);
> +  count= 0;
> +  while (!(rc= mysql_stmt_fetch(stmt)))
> +    count++;
> +  DIE_UNLESS(count == 0);
> +  rc= mysql_stmt_close(stmt);
> +  check_execute(stmt, rc);
> +
> +  rc= mysql_change_user(mysql, opt_user, opt_password, current_db);
> +  myquery(rc);

Another useless mysql_change_user.

> +}
>  
>  static struct my_tests_st my_tests[]= {
>    { "disable_query_logs", disable_query_logs },
> @@ -17790,6 +17897,7 @@ static struct my_tests_st my_tests[]= {
>    { "test_bug58036", test_bug58036 },
>    { "test_bug56976", test_bug56976 },
>    { "test_bug13001491", test_bug13001491 },
> +  { "test_mdev4326", test_mdev4326 },
>    { 0, 0 }
>  };

Regards,
Sergei