From: Date: January 7 2009 10:59am
Subject: Re: bzr commit into mysql-5.1-bugteam branch (davi:2739) Bug#41348
List-Archive: http://lists.mysql.com/commits/62578
Message-Id: <20090107095929.GA20165@janus.mylan>
MIME-Version: 1.0
Content-Type: text/plain; charset=koi8-r
Content-Transfer-Encoding: 8bit

Hi, Davi!

On Jan 06, Davi Arnaut wrote:
> On 1/6/09 11:35 AM, Sergei Golubchik wrote:
>> On Dec 17, Davi Arnaut wrote:
>>> # At a local mysql-5.1-bugteam repository of davi
>>>
>>>   2739 Davi Arnaut	2008-12-17
>>>        Bug#41348: INSERT INTO tbl SELECT * FROM temp_tbl overwrites
>>>                    locking type of temp table
>>>
>>>        The problem is that INSERT INTO .. SELECT FROM .. and CREATE
>>>        TABLE .. SELECT FROM a temporary table could inadvertently
>>>        overwrite the locking type of the temporary table. The lock
>>>        type of temporary tables should be a write lock by default.
>>
>> why here ?
>> You've added the code that overwrites tables->table->reginfo.lock_type,
>> perhaps you should fix what you've added ? Something like
>>
>> -    if (tables->lock_type != TL_UNLOCK&&  ! thd->locked_tables)
>> +    if (tables->lock_type != TL_UNLOCK&&  ! thd->locked_tables&&
>> +        tables->table->s->tmp_table == NO_TMP_TABLE)
>>
>> The intention of "NO_TMP_TABLE" check was to "skip lock_type update
>> for temporary tables", but apparently it was placed too late.
>
> I initially went with this approach, as can be seen in the first associated 
> to the bug, but it turns out that there a couple of other places that will 
> overwrite the lock_type without checking if its a temporary table.
...
> So, IMHO, this is a dormant problem (present in 5.0) that we need to 
> address in a more general way. Discussing the problem with Konstantin, we 
> seem to agree (kostja: chime in if I misunderstood) that temporary tables 
> shouldn't differ from base one in this regard and that a acceptable generic 
> solution is to reset the lock type of temporary tables when "closing" then.
> Do you agree? Suggestions?

Yes, I agree, it's much less error-prone approach.
But it looks inconsistent and confusing that in some places we check for
NO_TMP_TABLE but still restore the lock type because in other places we
don't check for it.

If you start restoring the lock type, then I suppose you should remove
NO_TMP_TABLE protection checks, right ?

Regards / Mit vielen Grüßen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Engineer/Server Architect
/_/  /_/\_, /___/\___\_\___/  Sun Microsystems GmbH, HRB München 161028
       <___/                  Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Häring