From: Sergey Vojtovich
Date: January 27 2011 3:15pm
Subject: Re: bzr commit into mysql-5.1 branch (nirbhay.choubey:3564)
 Bug#59398
List-Archive: http://lists.mysql.com/commits/129784
Message-Id: <20110127151530.GA26923@june>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Hi Nirbhay,

ok to push. There are a few minor suggestions inline.

On Mon, Jan 24, 2011 at 07:19:28PM +0000, Nirbhay Choubey wrote:
> #At file:///home/nirbhay/Project/mysql/repo/bugs/source/mysql-5.1.59398/ based on revid:martin.hansson@stripped
> 
>  3564 Nirbhay Choubey	2011-01-25
>       Bug#59398 : mysqldump 5.1 can't handle a dash ("-") in database
>                   names in ALTER DATABASE.
>       
>       mysqldump did not quote database name in 'ALTER DATABASE'
>       statements in its output. This can further cause a failure
>       while loading if database name contains a hyphen '-'.
>       
>       This happened as, while printing the 'ALTER DATABASE'
>       statements, the database name was not quoted.
>       
>       Fixed by quoting the database name.
>      @ client/mysqldump.c
>         Bug#59398 : mysqldump 5.1 can't handle a dash ("-") in database
>                     names in ALTER DATABASE.
>         
>         Modified the print statement in order to print the quoted
>         database name for 'ALTER DATABASE' statements.
>      @ mysql-test/r/mysqldump.result
>         Added a test case for bug#59398.
>      @ mysql-test/t/mysqldump.test
>         Added a test case for bug#59398.
> 
>     modified:
>       client/mysqldump.c
>       mysql-test/r/mysqldump.result
>       mysql-test/t/mysqldump.test
> === modified file 'client/mysqldump.c'
> --- a/client/mysqldump.c	2011-01-14 14:20:34 +0000
> +++ b/client/mysqldump.c	2011-01-24 19:19:24 +0000
> @@ -1134,6 +1134,9 @@ static int switch_db_collation(FILE *sql
>  {
>    if (strcmp(current_db_cl_name, required_db_cl_name) != 0)
>    {
> +    char quoted_database_buf[NAME_LEN*2+3];
Coding style says: "Put spaces both before and after ...
binary arithmetic operators ('+' etc.)".

> +    char *qdatabase= quote_name(db_name, quoted_database_buf, 1);
> +
We use three naming conventions here: db_name, quoted_database_buf,
qdatabase. I mean: "db" vs "database" and "q" vs "quoted_". I wish
some consistency here.

Coding style says: In C code, use TRUE and FALSE rather than 1/0.

I think third argument to quote_name() should be FALSE, so users
still have some control over quoting.

All of these comments are valid for restore_db_collation() as well.

...skip...

Regards,
Sergey
-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com