Leonard, hello.
The patch is good.
I have some comments.
Very important is to have test cases covering all situations the bug
is possible. I would suggest suite/rpl/t/rpl_sp.test as the tests
placeholder or a like.
> #At file:///home/zhl/mysql/rep/5.1/bug39526/
>
> 2793 Leonard Zhou 2009-02-12
> Bug#39526 Correct sql_mode in binary log for CREATE PROCEDURE
>
> sql_mode can't correctly write into binary log when creating a
> procedure.
I would say the sql_mode was just missed out. Could you please
formulate circumstances when it's so. In my view these are:
non-default sql_mode and one of CREATE trigger, stored function or
stored procedure.
That would be wonderful if check also if other CREATEable objects are
vulnerable (e.g EVENT) while you are within the current context.
>
> restore sql_mode before write events into log file when
> creating a procedure.
Ok.
Still I would comment that more specific. E.g restoring the current
session sql_mode right before generating the binlog event.
> modified:
> sql/sp.cc
>
> per-file messages:
> sql/sp.cc
> restore sql_mode before write events into log file
> === modified file 'sql/sp.cc'
> --- a/sql/sp.cc 2008-07-15 01:43:12 +0000
> +++ b/sql/sp.cc 2009-02-12 10:00:21 +0000
> @@ -937,6 +937,7 @@ sp_create_routine(THD *thd, int type, sp
> goto done;
> }
>
> + thd->variables.sql_mode= saved_mode;
> /* Such a statement can always go directly to binlog, no trans cache */
> thd->binlog_query(THD::MYSQL_QUERY_TYPE,
> log_query.c_ptr(), log_query.length(),
> FALSE, FALSE);
How about to return the mode back to the temporary value
+ thd->variables.sql_mode= 0;
in the same block which would look parenthesis-like?
In the end there will be always
thd->variables.sql_mode= saved_mode;
but we don't know future: if some more code will be added right
after `else if (mysql_bin_log.is_open()) {...}' lines that code
would assume thd->variables.sql_mode == 0 (overlooking your currently
correct restore).
Please provide your feedback, and i'll review your new patch asap.
cheers,
Andrei
