Sven, hej!
> Hi!
>
> Patch is good to push.
>
> I noticed something that would be good to document related to this. We
> are using an enumeration value to index an array. This puts a
> restriction on the order of the enumeration values which would be good
> to document. Also, we should hard-code the values of the enumeration
> constants. Something like this (patch against mysql-6.0-rpl):
Your concern is understandable. Eventually, sql_print_message_handlers
should be merged into MYSQL_ERROR class so that correspondence of the
array elements to the class' enum_warning_level will be provided
automatically.
>
>
> === modified file 'sql/log.cc'
> --- sql/log.cc 2008-06-10 22:27:52 +0000
> +++ sql/log.cc 2008-06-12 14:13:15 +0000
> @@ -98,6 +98,10 @@
> }
>
>
> +/*
> + Note that the indices of these elements must correspond to the
> + enumeration values of enum_warn_level.
> +*/
> sql_print_message_func sql_print_message_handlers[3] =
> {
> sql_print_information,
>
> === modified file 'sql/sql_error.h'
> --- sql/sql_error.h 2007-06-07 08:53:23 +0000
> +++ sql/sql_error.h 2008-06-12 15:03:45 +0000
> @@ -16,8 +16,15 @@
> class MYSQL_ERROR: public Sql_alloc
> {
> public:
> + /*
> + Enumeration value describing the severity of the error.
> +
> + Note that these enumeration values must correspond to the indices
> + of the sql_print_message_handlers array.
> + */
> enum enum_warning_level
> - { WARN_LEVEL_NOTE, WARN_LEVEL_WARN, WARN_LEVEL_ERROR, WARN_LEVEL_END};
> + { WARN_LEVEL_NOTE= 0, WARN_LEVEL_WARN= 1, WARN_LEVEL_ERROR= 2,
> + WARN_LEVEL_END};
>
> uint code;
> enum_warning_level level;
>
However, I think at this time we should not try merging.
Second, the comments are okay, but safer would be also to head
sql_print_message_func sql_print_message_handlers[3]
declaration. That would secure from uncontrolled (by MYSQL_ERROR
class) modification and provide the reference to the enum.
Third, not so important, still, WARN_LEVEL_NOTE= 0 is redundant as the
first item in an enum is implicitly zero (by the standard). Hence,
assignments are not needed.
How about the following extra comments?
=== modified file 'sql/log.cc'
--- sql/log.cc 2008-03-31 08:57:18 +0000
+++ sql/log.cc 2008-06-12 19:18:43 +0000
@@ -96,7 +96,13 @@ Silence_log_table_errors::handle_error(u
return TRUE;
}
+/*
+ Array of warning report functions.
+ The functions are listed in the order defined by
+ MYSQL_ERROR::enum_warning_level.
+ @todo: merge the array with MYSQL_ERROR class
+*/
sql_print_message_func sql_print_message_handlers[3] =
{
sql_print_information,
Once we have agreed on the last hunk, I'll commit and push.
cheers,
Andrei
>
>
>
> Andrei Elkin wrote:
>> #At file:///home/andrei/MySQL/BZR/FIXES/bug36968-rpl_temporary_errors_warn/
>>
>> 2612 Andrei Elkin 2008-06-05
>> Bug #36968 rpl_temporary_errors.test produces warning in pushbuild
>> The failure was due to incorrect type of the error
>> reporting function was invoked
>> at time the slave sql was about to leave.
>> Fixed with correcting that to use the type of logging
>> corrsponding to the error's level.
>> modified:
>> mysql-test/mysql-test-run.pl*
>> sql/slave.cc
>> support-files/build-tags
>>
>> per-file comments:
>> mysql-test/mysql-test-run.pl
>> permission to execute
>> sql/slave.cc
>> Error logging method corresponds to the level of the warning.
>> support-files/build-tags
>> tags start working.
>> === modified file 'mysql-test/mysql-test-run.pl' (properties changed: -x to +x)
>> === modified file 'sql/slave.cc'
>> --- a/sql/slave.cc 2008-03-31 08:57:18 +0000
>> +++ b/sql/slave.cc 2008-06-05 17:28:21 +0000
>> @@ -2757,7 +2757,8 @@
>> {
>> if (err->code == ER_CANT_OPEN_LIBRARY)
>> udf_error = true;
>> - sql_print_warning("Slave: %s Error_code: %d",err->msg,
> err->code);
>> + (sql_print_message_handlers[err->level])("Slave: %s Error_code:
> %d",
>> + err->msg, err->code);
>> }
>> if (udf_error)
>> sql_print_error("Error loading user-defined library, slave SQL "
>>
>> === modified file 'support-files/build-tags'
>> --- a/support-files/build-tags 2002-01-20 02:16:52 +0000
>> +++ b/support-files/build-tags 2008-06-05 17:28:21 +0000
>> @@ -2,7 +2,7 @@
>> rm -f TAGS
>> filter='\.cc$\|\.c$\|\.h$\|\.yy$'
>> -files=`bk -r sfiles -gU | grep $filter `
>> +files=`bzr ls | grep $filter `
>> for f in $files ;
>> do
>> etags -o TAGS --append $f
>>
>>
>
> --
> Sven Sandberg, Software Engineer
> MySQL AB, www.mysql.com
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe: http://lists.mysql.com/commits?unsub=1
>
