List:Commits« Previous MessageNext Message »
From:MATTIAS JONSSON Date:February 17 2011 9:39am
Subject:Re: bzr commit into mysql-trunk branch (jorgen.loland:3641) Bug#11762751
Bug#11764529
View as plain text  
Hi,

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() &&
table1->s->primary_key 
!= MAX_KEY)
#ifdef WITH_PARTITIONING_STORAGE_ENGINE
&& !(table1->part_info)
#endif
     )
   continue;

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.

/Mattias


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 
>>> anyway?
>>
>> 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
>> contrary.
>
> 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.)
Thread
bzr commit into mysql-trunk branch (jorgen.loland:3641) Bug#11762751Bug#11764529Jorgen Loland14 Feb
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3641) Bug#11762751Bug#11764529Jon Olav Hauglid15 Feb
    • Re: bzr commit into mysql-trunk branch (jorgen.loland:3641) Bug#11762751Bug#11764529Jorgen Loland15 Feb
      • Re: bzr commit into mysql-trunk branch (jorgen.loland:3641) Bug#11762751Bug#11764529Jon Olav Hauglid15 Feb
        • Re: bzr commit into mysql-trunk branch (jorgen.loland:3641) Bug#11762751Bug#11764529Jorgen Loland17 Feb
          • Re: bzr commit into mysql-trunk branch (jorgen.loland:3641) Bug#11762751Bug#11764529MATTIAS JONSSON17 Feb
            • Re: bzr commit into mysql-trunk branch (jorgen.loland:3641) Bug#11762751Bug#11764529Jorgen Loland17 Feb