Re: bk commit into 5.0 tree (mattiasj:1.2616) BUG#32575

[Mats Kindahl]

Hi Mattias!

Patch looks good. Patch approved.

Please check my comments inline below.

/Matz

mattiasj@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of mattiasj.  When mattiasj does a push these changes
> will be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2008-05-06 15:07:25+02:00,
> mattiasj@stripped +4 -0
>   Bug#32575 - Parse error of stmt with extended comments on slave side
>   
>   Problem was that mysql_create_view did not remove all comments characters
>   when writing to binlog, resulting in parse error of stmt on slave side.
>   
>   Solution was to use the recreated select clause
>   and add a generated CHECK OPTION clause if needed.
>
>   mysql-test/r/rpl_sp.result@stripped, 2008-05-06 15:07:23+02:00,
> mattiasj@stripped +1 -1
>     Bug#32575 - Parse error of stmt with extended comments on slave side
>     
>     Updated test result
>
>   mysql-test/r/rpl_view.result@stripped, 2008-05-06 15:07:23+02:00,
> mattiasj@stripped +27 -2
>     Bug#32575 - Parse error of stmt with extended comments on slave side
>     
>     Updated test result
>
>   mysql-test/t/rpl_view.test@stripped, 2008-05-06 15:07:23+02:00,
> mattiasj@stripped +25 -0
>     Bug#32575 - Parse error of stmt with extended comments on slave side
>     
>     Added test case
>
>   sql/sql_view.cc@stripped, 2008-05-06 15:07:23+02:00,
> mattiasj@stripped +5 -3
>     Bug#32575 - Parse error of stmt with extended comments on slave side
>     
>     Problem was that mysql_create_view did not remove all comments characters
>     when writing to binlog, resulting in parse error of stmt on slave side.
>     
>     Solution was to use the recreated select clause and generate
>     'WITH {LOCAL|CASCADED} CHECK OPTION'.
>
> diff -Nrup a/mysql-test/r/rpl_sp.result b/mysql-test/r/rpl_sp.result
> --- a/mysql-test/r/rpl_sp.result	2007-01-08 22:01:04 +01:00
> +++ b/mysql-test/r/rpl_sp.result	2008-05-06 15:07:23 +02:00
> @@ -499,7 +499,7 @@ fetch c into var;
>  close c;
>  return var;
>  end
> -master-bin.000001	#	Query	1	#	use `test`; CREATE ALGORITHM=UNDEFINED
> DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select 1 as a
> +master-bin.000001	#	Query	1	#	use `test`; CREATE ALGORITHM=UNDEFINED
> DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select 1 AS `a`
>  master-bin.000001	#	Query	1	#	use `test`; create table t1 (a int)
>  master-bin.000001	#	Query	1	#	use `test`; insert into t1 (a) values (f1())
>  master-bin.000001	#	Query	1	#	use `test`; drop view v1
> diff -Nrup a/mysql-test/r/rpl_view.result b/mysql-test/r/rpl_view.result
> --- a/mysql-test/r/rpl_view.result	2007-05-31 14:29:11 +02:00
> +++ b/mysql-test/r/rpl_view.result	2008-05-06 15:07:23 +02:00
> @@ -47,11 +47,11 @@ show binlog events limit 1,100;
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
>  slave-bin.000001	#	Query	1	#	use `test`; create table t1 (a int)
>  slave-bin.000001	#	Query	1	#	use `test`; insert into t1 values (1)
> -slave-bin.000001	#	Query	1	#	use `test`; CREATE ALGORITHM=UNDEFINED
> DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select a from t1
> +slave-bin.000001	#	Query	1	#	use `test`; CREATE ALGORITHM=UNDEFINED
> DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select `test`.`t1`.`a` AS `a`
> from `test`.`t1`
>  slave-bin.000001	#	Query	1	#	use `test`; insert into v1 values (2)
>  slave-bin.000001	#	Query	1	#	use `test`; update v1 set a=3 where a=1
>  slave-bin.000001	#	Query	1	#	use `test`; delete from v1 where a=2
> -slave-bin.000001	#	Query	1	#	use `test`; ALTER ALGORITHM=UNDEFINED
> DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select a as b from t1
> +slave-bin.000001	#	Query	1	#	use `test`; ALTER ALGORITHM=UNDEFINED
> DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select `test`.`t1`.`a` AS `b`
> from `test`.`t1`
>  slave-bin.000001	#	Query	1	#	use `test`; drop view v1
>  slave-bin.000001	#	Query	1	#	use `test`; drop table t1
>   

In general, we should try to remove all binlog event output from the 
rpl_* tests and store such things in a binlog_* test, but since you're 
just updating the result file this time, we'll leave it at that.

You might want to add a binlog_* test to check that the binary log 
really contains the queries without /*! ... */ comments.

>  
> @@ -109,6 +109,31 @@ drop view v1;
>  CREATE TABLE t1(a INT);
>  CREATE VIEW v1 AS SELECT * FROM t1;
>  CREATE VIEW v1 AS SELECT * FROM t1;
> +ERROR 42S01: Table 'v1' already exists
> +DROP VIEW v1;
> +DROP TABLE t1;
> +CREATE TABLE t1 (a INT);
> +# create view as output from mysqldump 10.11 (5.0.62)
> +/*!50001 DROP TABLE IF EXISTS `v1`*/;
> +Warnings:
> +Note	1051	Unknown table 'v1'
> +/*!50001 DROP VIEW IF EXISTS `v1`*/;
> +Warnings:
> +Note	1051	Unknown table 'test.v1'
> +/*!50001 CREATE ALGORITHM=UNDEFINED */
> +/*!50013 DEFINER=`root`@`localhost` SQL SECURITY DEFINER */
> +/*!50001 VIEW `v1` AS select `t1`.`a` AS `a` from `t1` where (`t1`.`a` < 3) */
> +/*!50002 WITH CASCADED CHECK OPTION */;
> +SHOW CREATE VIEW v1;
> +View	Create View
> +v1	CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW
> `v1` AS select `t1`.`a` AS `a` from `t1` where (`t1`.`a` < 3) WITH CASCADED CHECK
> OPTION
> +SHOW CREATE VIEW v1;
> +View	Create View
> +v1	CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW
> `v1` AS select `t1`.`a` AS `a` from `t1` where (`t1`.`a` < 3) WITH CASCADED CHECK
> OPTION
> +/*!50001 CREATE ALGORITHM=UNDEFINED */
> +/*!50013 DEFINER=`root`@`localhost` SQL SECURITY DEFINER */
> +/*!50001 VIEW `v1` AS select `t1`.`a` AS `a` from `t1` where (`t1`.`a` < 3) */
> +/*!50002 WITH CASCADED CHECK OPTION */;
>  ERROR 42S01: Table 'v1' already exists
>  DROP VIEW v1;
>  DROP TABLE t1;
> diff -Nrup a/mysql-test/t/rpl_view.test b/mysql-test/t/rpl_view.test
> --- a/mysql-test/t/rpl_view.test	2007-05-31 14:29:10 +02:00
> +++ b/mysql-test/t/rpl_view.test	2008-05-06 15:07:23 +02:00
> @@ -161,4 +161,29 @@ DROP VIEW v1;
>  DROP TABLE t1;
>  sync_slave_with_master;
>  
> +#
> +# Bug#32575 Parse error of stmt with extended comments on slave side
> +# comments characters was partly written to binlog
> +connection master;
> +CREATE TABLE t1 (a INT);
> +--echo # create view as output from mysqldump 10.11 (5.0.62)
> +/*!50001 DROP TABLE IF EXISTS `v1`*/;
> +/*!50001 DROP VIEW IF EXISTS `v1`*/;
> +/*!50001 CREATE ALGORITHM=UNDEFINED */
> +/*!50013 DEFINER=`root`@`localhost` SQL SECURITY DEFINER */
> +/*!50001 VIEW `v1` AS select `t1`.`a` AS `a` from `t1` where (`t1`.`a` < 3) */
> +/*!50002 WITH CASCADED CHECK OPTION */;
> +SHOW CREATE VIEW v1;
> +sync_slave_with_master;
> +SHOW CREATE VIEW v1;
> +connection master;
> +--error ER_TABLE_EXISTS_ERROR
> +/*!50001 CREATE ALGORITHM=UNDEFINED */
> +/*!50013 DEFINER=`root`@`localhost` SQL SECURITY DEFINER */
> +/*!50001 VIEW `v1` AS select `t1`.`a` AS `a` from `t1` where (`t1`.`a` < 3) */
> +/*!50002 WITH CASCADED CHECK OPTION */;
> +DROP VIEW v1;
> +DROP TABLE t1;
> +sync_slave_with_master;
> +
>  --echo End of 5.0 tests
> diff -Nrup a/sql/sql_view.cc b/sql/sql_view.cc
> --- a/sql/sql_view.cc	2008-02-21 18:58:27 +01:00
> +++ b/sql/sql_view.cc	2008-05-06 15:07:23 +02:00
> @@ -649,7 +649,11 @@ bool mysql_create_view(THD *thd, TABLE_L
>        buff.append(')');
>      }
>      buff.append(STRING_WITH_LEN(" AS "));
> -    buff.append(views->source.str, views->source.length);
> +    buff.append(views->query.str, views->query.length);
> +    if (views->with_check == VIEW_CHECK_LOCAL)
> +      buff.append(STRING_WITH_LEN(" WITH LOCAL CHECK OPTION"));
> +    else if (views->with_check == VIEW_CHECK_CASCADED)
> +      buff.append(STRING_WITH_LEN(" WITH CASCADED CHECK OPTION"));
>   

OK.

>  
>      Query_log_event qinfo(thd, buff.ptr(), buff.length(), 0, FALSE);
>      mysql_bin_log.write(&qinfo);
> @@ -926,8 +930,6 @@ loop_out:
>    }
>    DBUG_RETURN(0);
>  err:
> -  view->query.str= NULL;
> -  view->query.length= 0;
>    view->md5.str= NULL;
>    view->md5.length= 0;
>    DBUG_RETURN(error);
>
>   


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com