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

[Sven Sandberg]

Hi Mattias,

The source patch is good, I only have some small things in the test 
case: see below.

/Sven

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
>  
> @@ -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

Please write more explicitly what the purpose of the test is, i.e., the 
expected behavior (verify that 'CREATE VIEW' with comments is properly 
logged) rather than the problem with the bug.

> +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`*/;

I think the last two lines are unnecessary.

> +/*!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 */;

As we discussed on IRC, this part of the test can be removed.

> +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"));
>  
>      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);
> 

-- 
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com