Hi Lars-Erik,
On 11/16/09 9:12 AM, Lars-Erik.Bjork@stripped wrote:
> #At file:///home/lb200670/mysql/41569-mysql-5.1-bugteam/ based on
> revid:alexey.kopytov@stripped
>
> 3200 lars-erik.bjork@stripped 2009-11-16
> This is a patch for bug#41569.
> "mysql_upgrade (ver 5.1) add 3 fields to mysql.proc table but does
> not set values".
OK to push. Some minor comments below.
> mysql_upgrade (ver 5.1) adds 3 fields (character_set_client,
> collation_connection and db_collation) to the mysql.proc table, but
> does not set any values. When we run stored procedures, which were
> created with mysql 5.0, a warning is logged into the error log.
>
> The solution to this is for mysql_upgrade to set default best guess values
> for these fields. A warning is also written during upgrade, to make the
> user aware that default values are set.
> @ client/mysql_upgrade.c
> Result lines which start with "WARNING" are passed through to the output.
> This way we have a way of triggering WARNING-messages during upgrade
> directly from the .sql-script.
> @ mysql-test/r/mysql_upgrade.result
> Expected result of the test.
> @ mysql-test/t/mysql_upgrade.test
> Added a test-case for the bug.
> @ scripts/mysql_system_tables_fix.sql
> The new fields are populated, and warnings are written.
>
> modified:
> client/mysql_upgrade.c
> mysql-test/r/mysql_upgrade.result
> mysql-test/t/mysql_upgrade.test
> scripts/mysql_system_tables_fix.sql
> === modified file 'client/mysql_upgrade.c'
> --- a/client/mysql_upgrade.c 2009-09-28 06:24:19 +0000
> +++ b/client/mysql_upgrade.c 2009-11-16 11:12:19 +0000
> @@ -720,6 +720,15 @@ static my_bool is_expected_error(const c
> }
>
>
> +static my_bool is_warning(const char* line)
> +{
> + if (strncmp(line, "WARNING", 7) == 0)
> + return 1;
> + else
> + return 0;
> +}
> +
> +
> static char* get_line(char* line)
> {
> while (*line&& *line != '\n')
> @@ -778,6 +787,10 @@ static int run_sql_fix_privilege_tables(
> found_real_errors++;
> print_line(line);
> }
> + else if (is_warning(line))
No need for this function, call strncmp directly.
> + {
> + print_line(line);
> + }
Please use spaces as per our coding style.
> } while ((line= get_line(line))&& *line);
> }
>
>
> === modified file 'mysql-test/r/mysql_upgrade.result'
> --- a/mysql-test/r/mysql_upgrade.result 2009-01-26 14:20:33 +0000
> +++ b/mysql-test/r/mysql_upgrade.result 2009-11-16 11:12:19 +0000
> @@ -127,3 +127,45 @@ mysql.time_zone_transition
> mysql.time_zone_transition_type OK
> mysql.user OK
> set GLOBAL sql_mode=default;
> +#
> +# Bug #41569 mysql_upgrade (ver 5.1) add 3 fields to mysql.proc table
> +# but does not set values.
> +#
> +CREATE PROCEDURE testproc() BEGIN END;
> +UPDATE mysql.proc SET character_set_client = null where name like 'testproc';
> +UPDATE mysql.proc SET collation_connection = null where name like 'testproc';
> +UPDATE mysql.proc SET db_collation = null where name like 'testproc';
> +WARNING: Defaults set for mysql.proc.character_set_client fields with a value of
> null
> +WARNING: Defaults set for mysql.proc.collation_connection fields with a value of
> null
> +WARNING: Defaults set for mysql.proc.db_collation fields with a value of null
> +mtr.global_suppressions OK
> +mtr.test_suppressions OK
> +mysql.columns_priv OK
> +mysql.db OK
> +mysql.event OK
> +mysql.func OK
> +mysql.general_log
> +Error : You can't use locks with log tables.
> +status : OK
> +mysql.help_category OK
> +mysql.help_keyword OK
> +mysql.help_relation OK
> +mysql.help_topic OK
> +mysql.host OK
> +mysql.ndb_binlog_index OK
> +mysql.plugin OK
> +mysql.proc OK
> +mysql.procs_priv OK
> +mysql.servers OK
> +mysql.slow_log
> +Error : You can't use locks with log tables.
> +status : OK
> +mysql.tables_priv OK
> +mysql.time_zone OK
> +mysql.time_zone_leap_second OK
> +mysql.time_zone_name OK
> +mysql.time_zone_transition OK
> +mysql.time_zone_transition_type OK
> +mysql.user OK
> +CALL testproc();
> +DROP PROCEDURE testproc;
>
> === modified file 'mysql-test/t/mysql_upgrade.test'
> --- a/mysql-test/t/mysql_upgrade.test 2009-07-28 19:59:38 +0000
> +++ b/mysql-test/t/mysql_upgrade.test 2009-11-16 11:12:19 +0000
> @@ -89,3 +89,20 @@ DROP USER mysqltest1@'%';
> set GLOBAL sql_mode='STRICT_ALL_TABLES,ANSI_QUOTES,NO_ZERO_DATE';
> --exec $MYSQL_UPGRADE --skip-verbose --force 2>&1
> eval set GLOBAL sql_mode=default;
> +
> +
> +--echo #
> +--echo # Bug #41569 mysql_upgrade (ver 5.1) add 3 fields to mysql.proc table
> +--echo # but does not set values.
> +--echo #
> +
> +# Create a stored procedure and set the fields in question to null.
> +# When running mysql_upgrade, a warning should be written.
> +
> +CREATE PROCEDURE testproc() BEGIN END;
> +UPDATE mysql.proc SET character_set_client = null where name like 'testproc';
> +UPDATE mysql.proc SET collation_connection = null where name like 'testproc';
> +UPDATE mysql.proc SET db_collation = null where name like 'testproc';
Use uppercase properly in the query:
UPDATE mysql.proc SET db_collation = NULL WHERE name LIKE 'testproc';
> +--exec $MYSQL_UPGRADE --skip-verbose --force 2>&1
> +CALL testproc();
> +DROP PROCEDURE testproc;
>
> === modified file 'scripts/mysql_system_tables_fix.sql'
> --- a/scripts/mysql_system_tables_fix.sql 2009-10-27 10:09:36 +0000
> +++ b/scripts/mysql_system_tables_fix.sql 2009-11-16 11:12:19 +0000
> @@ -415,18 +415,48 @@ ALTER TABLE proc ADD character_set_clien
> ALTER TABLE proc MODIFY character_set_client
> char(32) collate utf8_bin DEFAULT NULL;
>
> +SELECT CASE WHEN COUNT(*)> 0 THEN
> +'WARNING: Defaults set for mysql.proc.character_set_client fields with a value of
> null'
The english looks like a bit strange. Perhaps something like:
CONCAT ("NULL values of the 'character_set_client' column ('mysql.proc'
table) have been updated with a default value (",
@@character_set_client, "). Please verify if necessary.");
Similar for the other warning messages.
> +ELSE null
> +END
> +AS value FROM proc where character_set_client is null;
Please see above comment about uppercase.
Regards,
--
Davi Arnaut