Your patch looks good. Patch approved (I vote for the change in
handler.cc, see below).
One review comment is to maybe add something like:
if (!(table1->file->primary_key_is_clustered() &&
In the first level loop to avoid doing the second level loop for tables
not matching the criteria.
To decrease a performance impact when many tables are used (o(n) vs
o(n*n)), but that is optional, since I don't think there will be too
many tables in mysql_multi_update_prepare anyway.
On 2011-02-17 09:55, Jorgen Loland wrote:
> On 02/15/2011 12:45 PM, Jon Olav Hauglid wrote:
>> On 02/15/2011 12:32 PM, Jorgen Loland wrote:
>>>> I think it would be good to also cover the behavior of an InnoDB table
>>>> without a primary key.
>>> InnoDB always has primary keys. If you don't provide one explicitly,
>>> InnoDB creates one on an autogenerated column. Is this a request
>> My thought was just to show that not all multi update statements on an
>> InnoDB table which uses the table twice, are disallowed.
> Ok, I will do that in a final patch after Mattias' review.
>>>> Is this change really needed anymore?
>>> No, but I got the impression from Mattias that this was something that
>>> was forgotten. I figured there could be other SQL statements that could
>>> also get into this error code. I don't have a strong opinion either way
>>> - remove or keep?
>> I vote to keep handler.cc unchanged, but I'm open to arguments to the
> Lets see what Mattias thinks :)
I vote FOR adding the error code to handler.cc and add a test for
showing this in MyISAM too :) (general errors from storage engines
should be translated to sql-errors if possible, which it is here.)