Hi Alfranio, Li-Bing,
Now I cannot parse myself ;).
Check below for a more clear sentence.
On Mon, 2009-09-21 at 21:29 +0100, Alfranio Correia wrote:
> Hi Libing,
>
>
> Great work.
> I agree with all the comments made by Luis.
>
> Cheers.
>
>
> Luís Soares wrote:
> > Hi Li-Bing,
> >
> > Nice Work. Please find my review comments below. I think we
> > just need some changes on where to place the test case. Then I
> > think it is ok to push.
> >
> > By the way, nice that you spotted the case for the extra DROP
> > TEMPORARY TABLE IF EXISTS when in RBL and no existed.
By the way, nice that you spotted the case for the extra DROP TEMPORARY
TABLE IF EXISTS 't' when in RBL and table 't' does not exist.
Maybe not perfect, but more readable.
:)
> >
> > STATUS
> > ------
> >
> > Not Approved.
> >
> > REQUIRED CHANGES
> > ----------------
> >
> > RC1. I think this test case should be moved to the binlog
> > suite. In fact, I think we can add it to the existing file
> > binlog_row_drop_tmp_tbl.test . Any objections?
> >
> > RC2. Please add some more comments to the test case (for
> > instance before the seconds drops to state what the test
> > is checking for).
> >
> > RC3. Rewrite the commit message in a more clear way. I can't
> > really parse it ;).
> >
> > REQUESTS
> > --------
> > n/a
> >
> > SUGGESTIONS
> > -----------
> > n/a
> >
> > DETAILS
> > -------
> > See some comments inline.
> >
> > On Mon, 2009-09-21 at 07:30 +0000, Li-Bing.Song@stripped wrote:
> >
> >> #At file:///home/anders/work/bzrwork/July/bug45574/mysql-5.1-bugteam/ based
> on revid:joro@stripped
> >>
> >> 3117 Li-Bing.Song@stripped 2009-09-21
> >> BUG #46572 DROP temp-table IF EXISTS does not have a consistent
> behavior in ROW mode
> >>
> >> In RBR, 'DROP TEMPORARY TABLE IF EXISTS...' statement is binlogged
> when the table
> >> is not exists.
> >> In RBR, 'DROP TEMPORARY TABLE ...' statement is never binlogged.
> >> This patch add an extra condition to fix the logic.
> >>
> >
> >
> > This message is a bit obscure. Can you please rewrite it in a more clear
> > way?
> >
> >
> >> added:
> >> mysql-test/suite/rpl/r/rpl_drop_temp_row.result
> >> mysql-test/suite/rpl/t/rpl_drop_temp_row.test
> >> modified:
> >> sql/sql_table.cc
> >> === added file 'mysql-test/suite/rpl/r/rpl_drop_temp_row.result'
> >> --- a/mysql-test/suite/rpl/r/rpl_drop_temp_row.result 1970-01-01 00:00:00
> +0000
> >> +++ b/mysql-test/suite/rpl/r/rpl_drop_temp_row.result 2009-09-21 07:30:43
> +0000
> >> @@ -0,0 +1,32 @@
> >> +stop slave;
> >> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> >> +reset master;
> >> +reset slave;
> >> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> >> +start slave;
> >> +DROP TABLE IF EXISTS tmp;
> >> +DROP TABLE IF EXISTS tmp1;
> >> +DROP TABLE IF EXISTS tmp2;
> >> +DROP TABLE IF EXISTS tmp3;
> >> +DROP TABLE IF EXISTS t;
> >> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp(c1 int);
> >> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp1(c1 int);
> >> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp2(c1 int);
> >> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp3(c1 int);
> >> +CREATE TABLE t(c1 int);
> >> +DROP TEMPORARY TABLE IF EXISTS tmp;
> >> +DROP TEMPORARY TABLE IF EXISTS tmp;
> >> +DROP TEMPORARY TABLE IF EXISTS tmp, tmp1;
> >> +DROP TABLE IF EXISTS tmp2, t;
> >> +DROP TABLE IF EXISTS tmp2, t;
> >> +DROP TEMPORARY TABLE tmp3;
> >> +show binlog events from <binlog_start>;
> >> +Log_name Pos Event_type Server_id End_log_pos Info
> >> +master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS tmp
> >> +master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS tmp1
> >> +master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS tmp2
> >> +master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS tmp3
> >> +master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS t
> >> +master-bin.000001 # Query # # use `test`; CREATE TABLE t(c1 int)
> >> +master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS `t` /*
> generated by server */
> >> +master-bin.000001 # Query # # use `test`; DROP TABLE IF EXISTS tmp2, t
> >>
> >> === added file 'mysql-test/suite/rpl/t/rpl_drop_temp_row.test'
> >> --- a/mysql-test/suite/rpl/t/rpl_drop_temp_row.test 1970-01-01 00:00:00
> +0000
> >> +++ b/mysql-test/suite/rpl/t/rpl_drop_temp_row.test 2009-09-21 07:30:43
> +0000
> >> @@ -0,0 +1,32 @@
> >>
> +#############################################################################
> >> +# BUG#46572 DROP temp-table IF EXISTS does not have a consistent behavior
> in
> >> +# ROW mode
> >> +#
> >> +# In RBR, 'DROP TEMPORARY TABLE ...' statement is always not binlogged.
> >>
> +#############################################################################
> >> +source include/master-slave.inc;
> >> +source include/have_binlog_format_row.inc;
> >> +disable_warnings;
> >> +
> >> +DROP TABLE IF EXISTS tmp;
> >> +DROP TABLE IF EXISTS tmp1;
> >> +DROP TABLE IF EXISTS tmp2;
> >> +DROP TABLE IF EXISTS tmp3;
> >> +DROP TABLE IF EXISTS t;
> >> +
> >> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp(c1 int);
> >> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp1(c1 int);
> >> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp2(c1 int);
> >> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp3(c1 int);
> >> +CREATE TABLE t(c1 int);
> >> +
> >> +DROP TEMPORARY TABLE IF EXISTS tmp;
> >> +DROP TEMPORARY TABLE IF EXISTS tmp;
> >> +DROP TEMPORARY TABLE IF EXISTS tmp, tmp1;
> >> +DROP TABLE IF EXISTS tmp2, t;
> >> +DROP TABLE IF EXISTS tmp2, t;
> >> +DROP TEMPORARY TABLE tmp3;
> >> +
> >> +source include/show_binlog_events.inc;
> >> +
> >> +source include/master-slave-end.inc;
> >>
> >> === modified file 'sql/sql_table.cc'
> >> --- a/sql/sql_table.cc 2009-09-18 13:01:18 +0000
> >> +++ b/sql/sql_table.cc 2009-09-21 07:30:43 +0000
> >> @@ -1949,7 +1949,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
> >> being built. The string always end in a comma and the comma
> >> will be chopped off before being written to the binary log.
> >> */
> >> - if (thd->current_stmt_binlog_row_based && !dont_log_query)
> >> + if (!drop_temporary && thd->current_stmt_binlog_row_based
> && !dont_log_query)
> >> {
> >> non_temp_tables_count++;
> >> /*
> >>
> >>
>
>
--
Luís
