Re: bk commit into 5.0 tree (cmiller:1.2572) BUG#34192

[Sergei Golubchik]

Hi!

On Feb 01, Chad MILLER wrote:
> ChangeSet@stripped, 2008-02-01 11:53:48-05:00, cmiller@stripped +1 -0
>   Bug#34192: mysqldump from mysql 5.0.51 silently fails on dumping \
>   	databases from 4.0 server
>   
>   Contribution from Arkadiusz Miskiewicz.  No CLA required for this size.
>   
>   mysqldump treated a failure to set the results charset as a severe
>   error.  
>   
>   Now, don't try to set the charset for the SHOW CREATE TABLE statement,
>   if the dumper doesn't want SET NAMES or the remote server doesn't 
>   support changing charsets.
>   
>   (The original patch tried to set the charset to binary and back in 
>   any case, and only exited-with-failure if the dumper wanted it and
>   the remote server supported it.)
> 
>   client/mysqldump.c@stripped, 2008-02-01 11:53:46-05:00, cmiller@stripped +13
> -3
>     Don't set the charset for receiving results if it's not wanted or if
>     the server doesn't support it.
> 
> diff -Nrup a/client/mysqldump.c b/client/mysqldump.c
> --- a/client/mysqldump.c	2007-12-04 22:07:00 -05:00
> +++ b/client/mysqldump.c	2008-02-01 11:53:46 -05:00
> @@ -1705,10 +1705,20 @@ static uint get_table_structure(char *ta
>  
>        my_snprintf(buff, sizeof(buff), "show create table %s", result_table);
>  
> -      if (switch_character_set_results(mysql, "binary") ||
> -          mysql_query_with_error_report(mysql, &result, buff) ||
> -          switch_character_set_results(mysql, default_charset))
> +      if (opt_set_charset)  /* Was forced to false if server is too old. */
> +      {
> +        if (switch_character_set_results(mysql, "binary") != 0)
> +          DBUG_RETURN(0);
> +      }
> +
> +      if (mysql_query_with_error_report(mysql, &result, buff) != 0)
>          DBUG_RETURN(0);
> +
> +      if (opt_set_charset)  /* Was forced to false if server is too old. */
> +      {
> +        if (switch_character_set_results(mysql, default_charset) != 0)
> +          DBUG_RETURN(0);
> +      }

No, this is wrong.

opt_charset specifies whether SET NAMES should be in the output.
It does not affect what is used for mysqldump<->server communications,
indeed, when a user can select what level of details he wants in the
dump, for communicating with the server we should always use the most
secure and robust settings.

The contributed patch was correct in this regard - it tried to switch
the charset, but ignored the error for old servers. Alternatively you
can only use switch_character_set_results() for new servers. But, again,
the options that configure the output do *not* affect how we communicate
with the server.

Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140