From: Sergey Vojtovich
Date: October 28 2010 12:10pm
Subject: Re: bzr commit into mysql-5.1-bugteam branch
 (nirbhay.choubey:3535) Bug#54899
List-Archive: http://lists.mysql.com/commits/122196
Message-Id: <20101028121008.GA3869@june>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Hi Nirbhay,

On Tue, Oct 19, 2010 at 10:47:50AM +0000, Nirbhay Choubey wrote:
> #At file:///home/nirbhay/Project/mysql/repo/wl/mysql-5.1-bugteam-54899/ based on revid:tor.didriksen@stripped
> 
>  3535 Nirbhay Choubey	2010-10-19
>       Bug #54899 : --one-database option cannot handle DROP/CREATE DATABASE commands
>       
>       After dropping and recreating the database specified along with --one-database
>       option at command line, mysql client keeps filtering the statements even after
>       the execution of a 'USE' command on the same database.
>       
>       --one-database option enables the filtering of statements when the current
>       database is not the one specified at the command line. However, when the same
>       database is dropped and recreated the variable (current_db) that holds the
>       inital database name gets altered. This bug exploits the fact that current_db
>       initially gets set to null value (0) when a 'use db_name' follows the recreation
>       of same database db_name (speficied at the command line) and hence skip_updates
>       gets set to 1, which inturn triggers the further filtering of statements.
(no action) Didn't understand much from the above para, but my brain was
close enough to get detonated. :)

>       Fixed by making get_current_db() a no-op function when one_database is set,
>       and hence, under that condition current_db will not get altered.
>       Note, however the value of current_db can change when we execute 'connect'
>       command with a differnet database to reconnect to the server, in which case,
>       the behavior of --one-database will be formulated using this new database.
>
>      @ client/mysql.cc
>         Bug #54899 : --one-database option cannot handle DROP/CREATE DATABASE commands
>         
>         Added an if statement at the beginnning of get_current_db() , which makes it
>         a no-op function if one-database option is specified, and hence current_db
>         remains unchanged.
(no action) Ehm, this comment is a copy of code you added.

>         Besides that, mysql client will now output a new warning 
>         message when a 'USE' command is executed with a different database.


>         Changed the help message for one-database option to a more approprite message
>         as specified in mysql documentation.
(no action) And this para is pretty good.

>      @ mysql-test/r/mysql.result
>         Added a test case for bug#54899 and a general test case to check one-database
>         option related functionality.
>      @ mysql-test/t/mysql.test
>         Added a test case for bug#54899 and a general test case to check one-database
>         option related functionality.
> 
>     modified:
>       client/mysql.cc
>       mysql-test/r/mysql.result
>       mysql-test/t/mysql.test
> === modified file 'client/mysql.cc'
> --- a/client/mysql.cc	2010-07-20 18:07:36 +0000
> +++ b/client/mysql.cc	2010-10-19 10:47:46 +0000
> @@ -1449,8 +1449,8 @@ static struct my_option my_long_options[
>     &opt_sigint_ignore,  &opt_sigint_ignore, 0, GET_BOOL,
>     NO_ARG, 0, 0, 0, 0, 0, 0},
>    {"one-database", 'o',
> -   "Only update the default database. This is useful for skipping updates "
> -   "to other database in the update log.",
> +   "Ignore statements except those that occur while the default "
> +   "database is the one named at the command line.",
>     0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0},
>  #ifdef USE_POPEN
>    {"pager", OPT_PAGER,
>
> @@ -2734,6 +2734,11 @@ static int reconnect(void)
>  
>  static void get_current_db()
>  {
> +  /*
> +    If one_database is set, current_db is not supposed to change.
> +  */
> +  if (one_database == 1)
> +    return;
A few suggestions:
- though in C++ it is ok to add code before variable declarations,
  it looks suspicious to me. Please do it after variable declaration.
- one_database is my_bool, I'd prefer to see "if (one_database)" instead.
- this comment seem to fit into one line perfectly.

>    MYSQL_RES *res;
>  
>    my_free(current_db, MYF(MY_ALLOW_ZERO_PTR));
> @@ -4112,6 +4117,11 @@ com_use(String *buffer __attribute__((un
>      {
>        skip_updates= 1;
>        select_db= 0;    // don't do mysql_select_db()
> +      put_info("Warning : MySQL client was started using one-database option. "
> +               "Hence, all further\nstatements will get filtered until 'USE' "
> +               "command is executed with the database\nspecified at command "
> +               "line.", INFO_INFO);
> +      return 0;
>      }
>      else
>        select_db= 2;    // do mysql_select_db() and build_completion_hash()
> 
Not sure. What if I my script is expecting "Database changed" here? After
all we pretend that "we change database and ignore statements", not
"we don't change database and ignore statements".

> === modified file 'mysql-test/r/mysql.result'
> --- a/mysql-test/r/mysql.result	2009-12-17 20:06:36 +0000
> +++ b/mysql-test/r/mysql.result	2010-10-19 10:47:46 +0000
> @@ -235,4 +235,26 @@ Bug #47147: mysql client option --skip-c
>  *************************** 1. row ***************************
>  1
>  
> +# Testing --one-database option
> +
> +CREATE DATABASE connected_db;
> +SHOW TABLES IN connected_db;
> +Tables_in_connected_db
> +t1
> +SHOW TABLES IN test;
> +Tables_in_test
> +t1
> +USE test;
> +DROP TABLE t1;
> +DROP DATABASE connected_db;
> +
> +Bug #54899: --one-database option cannot handle DROP/CREATE DATABASE commands
> +
> +CREATE DATABASE connected_db;
> +USE connected_db;
> +SHOW TABLES;
> +Tables_in_connected_db
> +t1
> +DROP DATABASE connected_db;
> +
>  End of tests
> 
> === modified file 'mysql-test/t/mysql.test'
> --- a/mysql-test/t/mysql.test	2009-12-17 20:06:36 +0000
> +++ b/mysql-test/t/mysql.test	2010-10-19 10:47:46 +0000
> @@ -413,4 +413,41 @@ drop table t1;
>  --exec $MYSQL --skip-column-names --vertical test -e "select 1 as a"
>  
>  --echo
> +--echo # Testing --one-database option
> +--echo
> +--write_file $MYSQLTEST_VARDIR/tmp/one_db.sql
> +CREATE TABLE t1 (i INT);
> +CREATE TABLE test.t1 (i INT);
> +USE test;
> +CREATE TABLE connected_db.t2 (i INT);
> +CREATE TABLE t2 (i INT);
> +EOF
> +
> +CREATE DATABASE connected_db;
> +--exec $MYSQL --one-database connected_db < $MYSQLTEST_VARDIR/tmp/one_db.sql
> +SHOW TABLES IN connected_db;
> +SHOW TABLES IN test;
> +USE test;
> +DROP TABLE t1;
> +DROP DATABASE connected_db;
> +--remove_file $MYSQLTEST_VARDIR/tmp/one_db.sql
> +
> +--echo
> +--echo Bug #54899: --one-database option cannot handle DROP/CREATE DATABASE commands
> +--echo
> +--write_file $MYSQLTEST_VARDIR/tmp/bug54899.sql
> +DROP DATABASE connected_db;
> +CREATE DATABASE connected_db;
> +USE connected_db;
> +CREATE TABLE t1(a INT);
> +EOF
> +
> +CREATE DATABASE connected_db;
> +--exec $MYSQL --one-database connected_db < $MYSQLTEST_VARDIR/tmp/bug54899.sql
> +USE connected_db;
> +SHOW TABLES;
> +DROP DATABASE connected_db;
> +--remove_file $MYSQLTEST_VARDIR/tmp/bug54899.sql
> +
> +--echo
>  --echo End of tests
> 


> 
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=svoj@stripped

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