Hi Horst,
nice trick to check for count(*)>4.
I'm fine with the patch in general. Please see below for some comments.
Horst Hunger, 02.09.2008 18:39:
...
> 2733 Horst Hunger 2008-09-02
> Fix for Bug#37766. Made the "failing" select save and fixed the testcase
> log_output='FILE'.
I am commenting on this patch, because it is references from the bug
report. I have seen later patches, but they don't appear in the bug
report and aren't complete fixes, but just updates. If some of my
comments are already fixed in your tree, please ignore them.
Regarding the revision comment, I would like to see the bug synopsis
with the bug number, and brief statements about "what was wrong" and
"how was it fixed".
There is a typo: save -> safe. (If you want to keep that obvious
sentence at all.)
What do you mean with "log_output='FILE'" ? Is it part of the sentence?
...
> --- a/mysql-test/r/log_output_func.result 2008-04-10 13:14:28 +0000
> +++ b/mysql-test/r/log_output_func.result 2008-09-02 16:39:17 +0000
...
> +SET @@global.general_log_file =
> '/work/bzr/mysql-5.1-sys-var/mysql-test/var/run/mytest.log';
The path name must not appear here.
...
> +connection default;
> +SET @@global.general_log= 'OFF';
> +SET @@global.general_log_file= @start_general_log_file;
> +SET @@global.general_log= 'ON';
> +SET @@global.log_output= @start_value;
> +SET @@global.general_log= @start_general_log;
Wouldn't it be better to turn on logging only after all log settings?
>
> === modified file 'mysql-test/t/log_output_func.test'
> --- a/mysql-test/t/log_output_func.test 2008-04-10 13:14:28 +0000
> +++ b/mysql-test/t/log_output_func.test 2008-09-02 16:39:17 +0000
...
> +eval SET @@global.general_log_file = '$MYSQLTEST_VARDIR/run/mytest.log';
Here we should suppress the path name with --replace_result, if it
helps, or with --disable_query_log + --enable_query_log.
...
> ####################################################
> # Endo of functionality Testing for log_output #
Typo (not yours): Endo -> End
...
OK to push after fixing the path name problem and optionally other of my
comments.
Regards
Ingo
--
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder, Wolfgang Engels, Dr. Roland Bömer
Vorsitzender des Aufsichtsrates: Martin Häring HRB München 161028
