List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:November 21 2009 12:41pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (lars-erik.bjork:3200)
Bug#41569
View as plain text  
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
Thread
bzr commit into mysql-5.1-bugteam branch (lars-erik.bjork:3200) Bug#41569lars-erik.bjork16 Nov
  • Re: bzr commit into mysql-5.1-bugteam branch (lars-erik.bjork:3200)Bug#41569Konstantin Osipov20 Nov
  • Re: bzr commit into mysql-5.1-bugteam branch (lars-erik.bjork:3200)Bug#41569Davi Arnaut21 Nov