Re: bzr commit into mysql-5.1-bugteam branch (davi:2739) Bug#41348

[Davi Arnaut]

Hi Sergei,

On 1/6/09 11:35 AM, Sergei Golubchik wrote:
> Hi, Davi!
>
> 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. Some will overwrite to another type of write lock 
(which is harmless), but others will overwrite it to a read lock. Take 
for example, this test case:

DELIMITER |;

CREATE FUNCTION f1() RETURNS INT
  BEGIN
      return 1 ;
  END|
DELIMITER ;|

CREATE TEMPORARY TABLE t1 (a INT);
CREATE TEMPORARY TABLE t2 (a INT);

UPDATE t1,t2 SET t1.a = t2.a;
INSERT INTO t2 SELECT f1();

During the prepare phase of the update in the above test case, the code 
below will mark tables that are only being read with a read lock type:

sql_update.cc:

1047├>      tl->lock_type= read_lock_type_for_table(thd, table);
1048│       tl->updating= 0;
1049│       /* Update TABLE::lock_type accordingly. */
1050│       if (!tl->placeholder() && !using_lock_tables)
1051│         tl->table->reginfo.lock_type= tl->lock_type;
1052│     }

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?

Regards,

-- Davi Arnaut