From: Jon Olav Hauglid
Date: January 17 2011 2:45pm
Subject: Re: bzr commit into mysql-5.5 branch (jorgen.loland:3256) Bug#55385
 Bug#57373
List-Archive: http://lists.mysql.com/commits/128984
Message-Id: <4D3455FF.2000008@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hello!

Review comments inline below.

On 01/17/2011 10:26 AM, Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-5.5/ based on revid:john.embretsen@stripped
>
>   3256 Jorgen Loland	2011-01-17
>        Bug#55385: UPDATE statement throws an error, but still updates the
>                   table entries
>        Bug#57373: Multi update+InnoDB reports ER_KEY_NOT_FOUND if a table
>                   is updated twice

As discussed, I think this patch should be made for Bug#55385 only
as Bug#57373 is also repeatable without partitioned tables.

>        The fix is to chech if multi-table update will

chech -> check

> === modified file 'sql/share/errmsg-utf8.txt'
> --- a/sql/share/errmsg-utf8.txt	2010-11-05 17:42:37 +0000
> +++ b/sql/share/errmsg-utf8.txt	2011-01-17 09:26:25 +0000
> @@ -6394,3 +6394,6 @@ ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE_MA
>
>   ER_STMT_CACHE_FULL
>           eng "Multi-row statements required more than 'max_binlog_stmt_cache_size' bytes of storage; increase this mysqld variable and try again"
> +
> +ER_MULTI_UPDATE_SAME_PARTITION
> +  eng "Cannot do multi-table update because partitioned table '%-.192s' is updated both by table/view '%-.192s' and '%-.192s'"
>

The documentation seems to use the term "multiple-table update".

Maybe rephrase the last part of the error message as
"is updated both as '%-.192s' and '%-.192s'" or something similar?
Since it's not two tables, just the same table named twice (either 
through aliases or views).

> === modified file 'sql/sql_update.cc'
> --- a/sql/sql_update.cc	2010-12-29 00:26:31 +0000
> +++ b/sql/sql_update.cc	2011-01-17 09:26:25 +0000
> @@ -1077,10 +1077,42 @@ int mysql_multi_update_prepare(THD *thd)

Please remove any trailing spaces.

Otherwise the patch looks fine to me.

Thanks!

--- Jon Olav