From 15ecf9228a3ba5e65f39cf87f941d0d997683dc6 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 23 Jan 2006 19:12:29 +0100 Subject: [PATCH 1/5] BUG#5390 - problems with merge tables After-fix optimizations proposed and finally implemented by Monty. mysql-test/r/lock.result: BUG#5390 - problems with merge tables After-fix optimizations proposed and finally implemented by Monty. Additional test results. mysql-test/t/lock.test: BUG#5390 - problems with merge tables After-fix optimizations proposed and finally implemented by Monty. Additional tests. sql/lock.cc: BUG#5390 - problems with merge tables After-fix optimizations proposed and finally implemented by Monty. get_lock_data() gets a flag for storing the lock positions in the new TABLE elements. mysql_lock_remove() can now remove a lock faster and more precisely as it has needed info in TABLE now. mysql_unlock_read_tables() and mysql_lock_merge() must now adjust the new elements of TABLE when modifying locks. mysql_lock_have_duplicate() can now work faster on the existing lock as the positions in the lock arrays are known for each table. get_lock_data() assigns the new TABLE elements on request of the new flag. sql/table.h: BUG#5390 - problems with merge tables After-fix optimizations proposed and finally implemented by Monty. Additional elements of TABLE. --- mysql-test/r/lock.result | 11 ++ mysql-test/t/lock.test | 20 ++++ sql/lock.cc | 235 +++++++++++++++++++++++++++------------ sql/table.h | 3 + 4 files changed, 198 insertions(+), 71 deletions(-) diff --git a/mysql-test/r/lock.result b/mysql-test/r/lock.result index 31a18fe6cec..923ab56d3af 100644 --- a/mysql-test/r/lock.result +++ b/mysql-test/r/lock.result @@ -47,3 +47,14 @@ unlock tables; lock tables t1 write, t1 as t1_alias read; insert into t1 select index1,nr from t1 as t1_alias; drop table t1,t2; +create table t1 (c1 int); +create table t2 (c1 int); +create table t3 (c1 int); +lock tables t1 write, t2 write, t3 write; +drop table t2, t3, t1; +create table t1 (c1 int); +create table t2 (c1 int); +create table t3 (c1 int); +lock tables t1 write, t2 write, t3 write, t1 as t4 read; +alter table t2 add column c2 int; +drop table t1, t2, t3; diff --git a/mysql-test/t/lock.test b/mysql-test/t/lock.test index 9015ce12fde..a7744319eb2 100644 --- a/mysql-test/t/lock.test +++ b/mysql-test/t/lock.test @@ -57,3 +57,23 @@ unlock tables; lock tables t1 write, t1 as t1_alias read; insert into t1 select index1,nr from t1 as t1_alias; drop table t1,t2; + +# +# BUG#5390 - problems with merge tables +# Supplement test for the after-fix optimization +# Check that a dropped table is correctly removed from a lock. +create table t1 (c1 int); +create table t2 (c1 int); +create table t3 (c1 int); +lock tables t1 write, t2 write, t3 write; +# This removes one table after the other from the lock. +drop table t2, t3, t1; +# +# Check that a lock merge works. +create table t1 (c1 int); +create table t2 (c1 int); +create table t3 (c1 int); +lock tables t1 write, t2 write, t3 write, t1 as t4 read; +alter table t2 add column c2 int; +drop table t1, t2, t3; + diff --git a/sql/lock.cc b/sql/lock.cc index 8944f23977f..bae5821eaf6 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -72,13 +72,16 @@ TODO: extern HASH open_cache; +/* flags for get_lock_data */ +#define GET_LOCK_UNLOCK 1 +#define GET_LOCK_STORE_LOCKS 2 + static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table,uint count, - bool unlock, TABLE **write_locked); + uint flags, TABLE **write_locked); static int lock_external(THD *thd, TABLE **table,uint count); static int unlock_external(THD *thd, TABLE **table,uint count); static void print_lock_error(int error); - /* Lock tables. @@ -104,7 +107,8 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count, uint flags) for (;;) { - if (!(sql_lock = get_lock_data(thd,tables,count, 0,&write_lock_used))) + if (! (sql_lock= get_lock_data(thd, tables, count, GET_LOCK_STORE_LOCKS, + &write_lock_used))) break; if (global_read_lock && write_lock_used && @@ -234,7 +238,8 @@ void mysql_unlock_some_tables(THD *thd, TABLE **table,uint count) { MYSQL_LOCK *sql_lock; TABLE *write_lock_used; - if ((sql_lock = get_lock_data(thd, table, count, 1, &write_lock_used))) + if ((sql_lock= get_lock_data(thd, table, count, GET_LOCK_UNLOCK, + &write_lock_used))) mysql_unlock_tables(thd, sql_lock); } @@ -271,6 +276,7 @@ void mysql_unlock_read_tables(THD *thd, MYSQL_LOCK *sql_lock) TABLE **table=sql_lock->table; for (i=found=0 ; i < sql_lock->table_count ; i++) { + DBUG_ASSERT(sql_lock->table[i]->lock_position == i); if ((uint) sql_lock->table[i]->reginfo.lock_type >= TL_WRITE_ALLOW_READ) { swap(TABLE *,*table,sql_lock->table[i]); @@ -284,6 +290,17 @@ void mysql_unlock_read_tables(THD *thd, MYSQL_LOCK *sql_lock) VOID(unlock_external(thd,table,i-found)); sql_lock->table_count=found; } + /* Fix the lock positions in TABLE */ + table= sql_lock->table; + found= 0; + for (i= 0; i < sql_lock->table_count; i++) + { + TABLE *tbl= *table; + tbl->lock_position= table - sql_lock->table; + tbl->lock_data_start= found; + found+= tbl->lock_count; + table++; + } DBUG_VOID_RETURN; } @@ -299,20 +316,51 @@ void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table) { if (locked->table[i] == table) { - locked->table_count--; + uint j, removed_locks, old_tables; + TABLE *tbl; + uint lock_data_end; + + DBUG_ASSERT(table->lock_position == i); + + /* Decrement table_count in advance, making below expressions easier */ + old_tables= --locked->table_count; + + /* The table has 'removed_locks' lock data elements in locked->locks */ + removed_locks= table->lock_count; + + /* Move down all table pointers above 'i'. */ bmove((char*) (locked->table+i), (char*) (locked->table+i+1), - (locked->table_count-i)* sizeof(TABLE*)); + (old_tables - i) * sizeof(TABLE*)); + + lock_data_end= table->lock_data_start + table->lock_count; + /* Move down all lock data pointers above 'table->lock_data_end-1' */ + bmove((char*) (locked->locks + table->lock_data_start), + (char*) (locked->locks + lock_data_end), + (locked->lock_count - lock_data_end) * + sizeof(THR_LOCK_DATA*)); + + /* + Fix moved table elements. + lock_position is the index in the 'locked->table' array, + it must be fixed by one. + table->lock_data_start is pointer to the lock data for this table + in the 'locked->locks' array, they must be fixed by 'removed_locks', + the lock data count of the removed table. + */ + for (j= i ; j < old_tables; j++) + { + tbl= locked->table[j]; + tbl->lock_position--; + DBUG_ASSERT(tbl->lock_position == j); + tbl->lock_data_start-= removed_locks; + } + + /* Finally adjust lock_count. */ + locked->lock_count-= removed_locks; break; } } - THR_LOCK_DATA **prev=locked->locks; - for (i=0 ; i < locked->lock_count ; i++) - { - if (locked->locks[i]->type != TL_UNLOCK) - *prev++ = locked->locks[i]; - } - locked->lock_count=(uint) (prev - locked->locks); } } @@ -322,7 +370,8 @@ void mysql_lock_abort(THD *thd, TABLE *table) { MYSQL_LOCK *locked; TABLE *write_lock_used; - if ((locked = get_lock_data(thd,&table,1,1,&write_lock_used))) + if ((locked= get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK, + &write_lock_used))) { for (uint i=0; i < locked->lock_count; i++) thr_abort_locks(locked->locks[i]->lock); @@ -351,7 +400,8 @@ bool mysql_lock_abort_for_thread(THD *thd, TABLE *table) bool result= FALSE; DBUG_ENTER("mysql_lock_abort_for_thread"); - if ((locked = get_lock_data(thd,&table,1,1,&write_lock_used))) + if ((locked= get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK, + &write_lock_used))) { for (uint i=0; i < locked->lock_count; i++) { @@ -368,7 +418,9 @@ bool mysql_lock_abort_for_thread(THD *thd, TABLE *table) MYSQL_LOCK *mysql_lock_merge(MYSQL_LOCK *a,MYSQL_LOCK *b) { MYSQL_LOCK *sql_lock; + TABLE **table, **end_table; DBUG_ENTER("mysql_lock_merge"); + if (!(sql_lock= (MYSQL_LOCK*) my_malloc(sizeof(*sql_lock)+ sizeof(THR_LOCK_DATA*)*(a->lock_count+b->lock_count)+ @@ -384,6 +436,21 @@ MYSQL_LOCK *mysql_lock_merge(MYSQL_LOCK *a,MYSQL_LOCK *b) memcpy(sql_lock->table,a->table,a->table_count*sizeof(*a->table)); memcpy(sql_lock->table+a->table_count,b->table, b->table_count*sizeof(*b->table)); + + /* + Now adjust lock_position and lock_data_start for all objects that was + moved in 'b' (as there is now all objects in 'a' before these). + */ + for (table= sql_lock->table + a->table_count, + end_table= table + b->table_count; + table < end_table; + table++) + { + (*table)->lock_position+= a->table_count; + (*table)->lock_data_start+= a->lock_count; + } + + /* Delete old, not needed locks */ my_free((gptr) a,MYF(0)); my_free((gptr) b,MYF(0)); DBUG_RETURN(sql_lock); @@ -402,73 +469,78 @@ MYSQL_LOCK *mysql_lock_merge(MYSQL_LOCK *a,MYSQL_LOCK *b) NOTE This is mainly meant for MERGE tables in INSERT ... SELECT situations. The 'real', underlying tables can be found only after - the table is opened. The easier way is to check this after the - tables are locked. + the MERGE tables are opened. This function assumes that the tables are + already locked. + + Temporary tables are ignored here like they are ignored in + get_lock_data(). If we allow two opens on temporary tables later, + both functions should be checked. RETURN 1 A table from 'tables' matches a lock on 'table'. - 0 No duplicate lock is present. - -1 Error. + 0 No duplicate lock found. */ int mysql_lock_have_duplicate(THD *thd, TABLE *table, TABLE_LIST *tables) { - uint count; - MYSQL_LOCK *sql_lock1; - MYSQL_LOCK *sql_lock2; - TABLE **tables1= &table; - TABLE **tables2; - TABLE **table_ptr; - TABLE_LIST *tablist2; - TABLE *write_lock_used; - THR_LOCK_DATA **lock_data1; - THR_LOCK_DATA **end_data1; + MYSQL_LOCK *mylock; + TABLE **lock_tables; + TABLE *table2; + THR_LOCK_DATA **lock_locks, **table_lock_data; + THR_LOCK_DATA **end_data; THR_LOCK_DATA **lock_data2; THR_LOCK_DATA **end_data2; - THR_LOCK *lock1; DBUG_ENTER("mysql_lock_have_duplicate"); - if (! (sql_lock1= get_lock_data(thd, tables1, 1, 1, &write_lock_used))) - goto err0; + /* A temporary table does not have locks. */ + if (table->tmp_table == TMP_TABLE) + goto end; - count=0; - for (tablist2 = tables; tablist2; tablist2= tablist2->next) - count++; - if (! (tables2= (TABLE**) sql_alloc(sizeof(TABLE*) * count))) - goto err1; - table_ptr= tables2; - for (tablist2 = tables; tablist2; tablist2= tablist2->next) - *(table_ptr++)= tablist2->table; - if (! (sql_lock2= get_lock_data(thd, tables2, count, 1, &write_lock_used))) - goto err1; + /* Get command lock or LOCK TABLES lock. */ + mylock= thd->lock ? thd->lock : thd->locked_tables; + DBUG_ASSERT(mylock); - count= 1; - for (lock_data1= sql_lock1->locks, - end_data1= lock_data1 + sql_lock1->lock_count; - lock_data1 < end_data1; - lock_data1++) + /* If we have less than two tables, we cannot have duplicates. */ + if (mylock->table_count < 2) + goto end; + + lock_locks= mylock->locks; + lock_tables= mylock->table; + + /* Prepare table related variables that don't change in loop. */ + DBUG_ASSERT(table == lock_tables[table->lock_position]); + table_lock_data= lock_locks + table->lock_data_start; + end_data= table_lock_data + table->lock_count; + + for (; tables; tables= tables->next) { - lock1= (*lock_data1)->lock; - for (lock_data2= sql_lock2->locks, - end_data2= lock_data2 + sql_lock2->lock_count; + table2= tables->table; + if (table2->tmp_table == TMP_TABLE) + continue; + + /* All tables in list must be in lock. */ + DBUG_ASSERT(table2 == lock_tables[table2->lock_position]); + + for (lock_data2= lock_locks + table2->lock_data_start, + end_data2= lock_data2 + table2->lock_count; lock_data2 < end_data2; lock_data2++) { - if ((*lock_data2)->lock == lock1) - goto end; + THR_LOCK_DATA **lock_data; + THR_LOCK *lock2= (*lock_data2)->lock; + + for (lock_data= table_lock_data; + lock_data < end_data; + lock_data++) + { + if ((*lock_data)->lock == lock2) + DBUG_RETURN(1); + } } } - count= 0; end: - my_free((gptr) sql_lock2, MYF(0)); - my_free((gptr) sql_lock1, MYF(0)); - DBUG_RETURN(count); - - err1: - my_free((gptr) sql_lock1, MYF(0)); - err0: - DBUG_RETURN(-1); + DBUG_RETURN(0); } @@ -497,17 +569,27 @@ static int unlock_external(THD *thd, TABLE **table,uint count) /* -** Get lock structures from table structs and initialize locks + Get lock structures from table structs and initialize locks + + SYNOPSIS + get_lock_data() + thd Thread handler + table_ptr Pointer to tables that should be locks + flags One of: + GET_LOCK_UNLOCK: If we should send TL_IGNORE to + store lock + GET_LOCK_STORE_LOCKS: Store lock info in TABLE + write_lock_used Store pointer to last table with WRITE_ALLOW_WRITE */ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, - bool get_old_locks, TABLE **write_lock_used) + uint flags, TABLE **write_lock_used) { uint i,tables,lock_count; MYSQL_LOCK *sql_lock; - THR_LOCK_DATA **locks; - TABLE **to; + THR_LOCK_DATA **locks, **locks_buf, **locks_start; + TABLE **to, **table_buf; DBUG_ENTER("get_lock_data"); *write_lock_used=0; @@ -525,18 +607,19 @@ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, sizeof(THR_LOCK_DATA*)*tables+sizeof(table_ptr)*lock_count, MYF(0)))) DBUG_RETURN(0); - locks=sql_lock->locks=(THR_LOCK_DATA**) (sql_lock+1); - to=sql_lock->table=(TABLE**) (locks+tables); + locks= locks_buf= sql_lock->locks= (THR_LOCK_DATA**) (sql_lock + 1); + to= table_buf= sql_lock->table= (TABLE**) (locks + tables); sql_lock->table_count=lock_count; sql_lock->lock_count=tables; for (i=0 ; i < count ; i++) { TABLE *table; + enum thr_lock_type lock_type; + if ((table=table_ptr[i])->tmp_table == TMP_TABLE) continue; - *to++=table; - enum thr_lock_type lock_type= table->reginfo.lock_type; + lock_type= table->reginfo.lock_type; if (lock_type >= TL_WRITE_ALLOW_WRITE) { *write_lock_used=table; @@ -547,12 +630,22 @@ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, DBUG_RETURN(0); } } - locks=table->file->store_lock(thd, locks, get_old_locks ? TL_IGNORE : - lock_type); + locks_start= locks; + locks= table->file->store_lock(thd, locks, + (flags & GET_LOCK_UNLOCK) ? TL_IGNORE : + lock_type); + if (flags & GET_LOCK_STORE_LOCKS) + { + table->lock_position= (uint) (to - table_buf); + table->lock_data_start= (uint) (locks_start - locks_buf); + table->lock_count= (uint) (locks - locks_start); + } + *to++= table; } DBUG_RETURN(sql_lock); } + /***************************************************************************** ** Lock table based on the name. ** This is used when we need total access to a closed, not open table diff --git a/sql/table.h b/sql/table.h index 0a1d1893531..610c76e35c2 100644 --- a/sql/table.h +++ b/sql/table.h @@ -86,6 +86,9 @@ struct st_table { uint next_number_index; uint blob_ptr_size; /* 4 or 8 */ uint next_number_key_offset; + uint lock_position; /* Position in MYSQL_LOCK.table */ + uint lock_data_start; /* Start pos. in MYSQL_LOCK.locks */ + uint lock_count; /* Number of locks */ int current_lock; /* Type of lock on table */ enum tmp_table_type tmp_table; my_bool copy_blobs; /* copy_blobs when storing */ From 4646cc587f86596a8e8f432332fe59ecdb598a5a Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 20 Feb 2006 12:28:07 +0100 Subject: [PATCH 2/5] BUG#5390 - problems with merge tables This fix is necessary because thr_multi_lock() reorderes the lock data references array. --- sql/lock.cc | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/sql/lock.cc b/sql/lock.cc index bae5821eaf6..713eb02d8ec 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -141,7 +141,12 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count, uint flags) } thd->proc_info="Table lock"; thd->locked=1; - if (thr_multi_lock(sql_lock->locks,sql_lock->lock_count)) + /* Copy the lock data array. thr_multi_lock() reorders its contens. */ + memcpy(sql_lock->locks + sql_lock->lock_count, sql_lock->locks, + sql_lock->lock_count * sizeof(*sql_lock->locks)); + /* Lock on the copied half of the lock data array. */ + if (thr_multi_lock(sql_lock->locks + sql_lock->lock_count, + sql_lock->lock_count)) { thd->some_tables_deleted=1; // Try again sql_lock->lock_count=0; // Locks are alread freed @@ -602,13 +607,20 @@ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, } } + /* + Allocating twice the number of pointers for lock data for use in + thr_mulit_lock(). This function reorders the lock data, but cannot + update the table values. So the second part of the array is copied + from the first part immediately before calling thr_multi_lock(). + */ if (!(sql_lock= (MYSQL_LOCK*) - my_malloc(sizeof(*sql_lock)+ - sizeof(THR_LOCK_DATA*)*tables+sizeof(table_ptr)*lock_count, + my_malloc(sizeof(*sql_lock) + + sizeof(THR_LOCK_DATA*) * tables * 2 + + sizeof(table_ptr) * lock_count, MYF(0)))) DBUG_RETURN(0); locks= locks_buf= sql_lock->locks= (THR_LOCK_DATA**) (sql_lock + 1); - to= table_buf= sql_lock->table= (TABLE**) (locks + tables); + to= table_buf= sql_lock->table= (TABLE**) (locks + tables * 2); sql_lock->table_count=lock_count; sql_lock->lock_count=tables; From ab16adb7dd56106bf73172799fdbe8ce6809adab Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 20 Feb 2006 15:23:57 +0100 Subject: [PATCH 3/5] BUG#5390 - problems with merge tables Merge from 4.1 --- sql/lock.cc | 143 +++++++++++++++++++++++----------------------------- sql/table.h | 17 +++---- 2 files changed, 68 insertions(+), 92 deletions(-) diff --git a/sql/lock.cc b/sql/lock.cc index 6f206f017e4..bff7cfd6734 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -499,112 +499,91 @@ MYSQL_LOCK *mysql_lock_merge(MYSQL_LOCK *a,MYSQL_LOCK *b) NOTE This is mainly meant for MERGE tables in INSERT ... SELECT situations. The 'real', underlying tables can be found only after - the table is opened. The easier way is to check this after the - tables are locked. + the MERGE tables are opened. This function assumes that the tables are + already locked. + + Temporary tables are ignored here like they are ignored in + get_lock_data(). If we allow two opens on temporary tables later, + both functions should be checked. RETURN - 1 A table from 'tables' matches a lock on 'table'. - 0 No duplicate lock is present. - -1 Error. + NULL No duplicate lock found. + ! NULL First table from 'haystack' that matches a lock on 'needle'. */ TABLE_LIST *mysql_lock_have_duplicate(THD *thd, TABLE_LIST *needle, TABLE_LIST *haystack) { - uint count; - uint dup_pos; - TABLE *write_lock_used; /* dummy */ - TABLE **tables1; - TABLE **tables2; - TABLE **table_ptr; - TABLE_LIST *tlist_ptr; - MYSQL_LOCK *sql_lock1; - MYSQL_LOCK *sql_lock2; - THR_LOCK_DATA **lock_data1; - THR_LOCK_DATA **end_data1; + MYSQL_LOCK *mylock; + TABLE **lock_tables; + TABLE *table; + TABLE *table2; + THR_LOCK_DATA **lock_locks; + THR_LOCK_DATA **table_lock_data; + THR_LOCK_DATA **end_data; THR_LOCK_DATA **lock_data2; THR_LOCK_DATA **end_data2; - THR_LOCK *lock1; DBUG_ENTER("mysql_lock_have_duplicate"); /* Table may not be defined for derived or view tables. */ - if (! needle->table) - DBUG_RETURN(NULL); + if (! (table= needle->table)) + goto end; - /* Get lock(s) for needle. */ - tables1= &needle->table; - if (! (sql_lock1= get_lock_data(thd, tables1, 1, 1, &write_lock_used))) - goto err0; + /* A temporary table does not have locks. */ + if (table->s->tmp_table == TMP_TABLE) + goto end; - /* Count real tables in list. */ - count=0; - for (tlist_ptr = haystack; tlist_ptr; tlist_ptr= tlist_ptr->next_global) - if (! tlist_ptr->placeholder() && ! tlist_ptr->schema_table) - count++; - /* Allocate a table array. */ - if (! (tables2= (TABLE**) sql_alloc(sizeof(TABLE*) * count))) - goto err1; - table_ptr= tables2; - /* Assign table pointers. */ - for (tlist_ptr = haystack; tlist_ptr; tlist_ptr= tlist_ptr->next_global) - if (! tlist_ptr->placeholder() && ! tlist_ptr->schema_table) - *(table_ptr++)= tlist_ptr->table; - /* Get lock(s) for haystack. */ - if (! (sql_lock2= get_lock_data(thd, tables2, count, 1, &write_lock_used))) - goto err1; + /* Get command lock or LOCK TABLES lock. Maybe empty for INSERT DELAYED. */ + if (! (mylock= thd->lock ? thd->lock : thd->locked_tables)) + goto end; - /* Initialize duplicate position to an impossible value. */ - dup_pos= UINT_MAX; - /* - Find a duplicate lock. - In case of merge tables, sql_lock1 can have more than 1 lock. - */ - for (lock_data1= sql_lock1->locks, - end_data1= lock_data1 + sql_lock1->lock_count; - lock_data1 < end_data1; - lock_data1++) + /* If we have less than two tables, we cannot have duplicates. */ + if (mylock->table_count < 2) + goto end; + + lock_locks= mylock->locks; + lock_tables= mylock->table; + + /* Prepare table related variables that don't change in loop. */ + DBUG_ASSERT(table == lock_tables[table->lock_position]); + table_lock_data= lock_locks + table->lock_data_start; + end_data= table_lock_data + table->lock_count; + + for (; haystack; haystack= haystack->next_global) { - lock1= (*lock_data1)->lock; - for (lock_data2= sql_lock2->locks, - end_data2= lock_data2 + sql_lock2->lock_count; + if (haystack->placeholder() || haystack->schema_table) + continue; + table2= haystack->table; + if (table2->s->tmp_table == TMP_TABLE) + continue; + + /* All tables in list must be in lock. */ + DBUG_ASSERT(table2 == lock_tables[table2->lock_position]); + + for (lock_data2= lock_locks + table2->lock_data_start, + end_data2= lock_data2 + table2->lock_count; lock_data2 < end_data2; lock_data2++) { - if ((*lock_data2)->lock == lock1) + THR_LOCK_DATA **lock_data; + THR_LOCK *lock2= (*lock_data2)->lock; + + for (lock_data= table_lock_data; + lock_data < end_data; + lock_data++) { - DBUG_PRINT("ingo", ("duplicate lock found")); - /* Change duplicate position to the real value. */ - dup_pos= lock_data2 - sql_lock2->locks; - goto end; + if ((*lock_data)->lock == lock2) + { + DBUG_PRINT("info", ("haystack match: '%s'", haystack->table_name)); + DBUG_RETURN(haystack); + } } } } end: - tlist_ptr= NULL; /* In case that no duplicate was found. */ - if (dup_pos != UINT_MAX) - { - /* Duplicate found. Search the matching TABLE_LIST object. */ - count= 0; - for (tlist_ptr = haystack; tlist_ptr; tlist_ptr= tlist_ptr->next_global) - { - if (! tlist_ptr->placeholder() && ! tlist_ptr->schema_table) - { - count+= tlist_ptr->table->file->lock_count(); - if (count > dup_pos) - break; - } - } - } - my_free((gptr) sql_lock2, MYF(0)); - my_free((gptr) sql_lock1, MYF(0)); - DBUG_RETURN(tlist_ptr); - - err1: - my_free((gptr) sql_lock1, MYF(0)); - err0: - /* This non-null but special value indicates error, if caller cares. */ - DBUG_RETURN(needle); + DBUG_PRINT("info", ("no duplicate found")); + DBUG_RETURN(NULL); } @@ -693,6 +672,8 @@ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, for (i=0 ; i < count ; i++) { TABLE *table; + enum thr_lock_type lock_type; + if ((table=table_ptr[i])->s->tmp_table == TMP_TABLE) continue; lock_type= table->reginfo.lock_type; diff --git a/sql/table.h b/sql/table.h index 62990a966cc..9d44d136773 100644 --- a/sql/table.h +++ b/sql/table.h @@ -234,15 +234,10 @@ struct st_table { */ timestamp_auto_set_type timestamp_field_type; table_map map; /* ID bit of table (1,2,4,8,16...) */ - - uint next_number_index; - uint blob_ptr_size; /* 4 or 8 */ - uint next_number_key_offset; - uint lock_position; /* Position in MYSQL_LOCK.table */ - uint lock_data_start; /* Start pos. in MYSQL_LOCK.locks */ - uint lock_count; /* Number of locks */ - int current_lock; /* Type of lock on table */ - enum tmp_table_type tmp_table; + + uint lock_position; /* Position in MYSQL_LOCK.table */ + uint lock_data_start; /* Start pos. in MYSQL_LOCK.locks */ + uint lock_count; /* Number of locks */ uint tablenr,used_fields; uint temp_pool_slot; /* Used by intern temp tables */ uint status; /* What's in record[0] */ @@ -251,8 +246,8 @@ struct st_table { uint derived_select_number; int current_lock; /* Type of lock on table */ my_bool copy_blobs; /* copy_blobs when storing */ - - /* + + /* 0 or JOIN_TYPE_{LEFT|RIGHT}. Currently this is only compared to 0. If maybe_null !=0, this table is inner w.r.t. some outer join operation, and null_row may be true. From 10876da86467fc891e05bec9a1b6fdfd186a80ba Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 20 Feb 2006 18:04:50 +0100 Subject: [PATCH 4/5] BUG#5390 - problems with merge tables Fix for merge from 4.1. --- sql/lock.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/sql/lock.cc b/sql/lock.cc index 22c3b03f8c1..01cdca0b6ee 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -159,8 +159,6 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count, memcpy(sql_lock->locks + sql_lock->lock_count, sql_lock->locks, sql_lock->lock_count * sizeof(*sql_lock->locks)); /* Lock on the copied half of the lock data array. */ - if (thr_multi_lock(sql_lock->locks + sql_lock->lock_count, - sql_lock->lock_count)) rc= thr_lock_errno_to_mysql[(int) thr_multi_lock(sql_lock->locks + sql_lock->lock_count, sql_lock->lock_count, From 23549493aad7f078dc86adc912c1b0b2b06c8568 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 5 Apr 2006 14:39:20 +0200 Subject: [PATCH 5/5] BUG#5390 - problems with merge tables Additional fix for INSERT DELAYED with subselect. Originally detected in 5.1, but 5.0 could also be affected. The user thread creates a dummy table object, which is not included in the lock. The 'real' table is opened and locked by the 'delayed' system thread. The dummy object is now marked as not locked and this is tested in mysql_lock_have_duplicate(). sql/lock.cc: BUG#5390 - problems with merge tables Additional fix for INSERT DELAYED with subselect. The user thread creates a dummy table object, which is not included in the lock. Changed to safer asserts. sql/sql_insert.cc: BUG#5390 - problems with merge tables Marked the dummy table object as not being included in the lock. --- sql/lock.cc | 13 +++++++++---- sql/sql_insert.cc | 5 ++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/sql/lock.cc b/sql/lock.cc index 01cdca0b6ee..a584b063a0b 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -530,8 +530,11 @@ TABLE_LIST *mysql_lock_have_duplicate(THD *thd, TABLE_LIST *needle, THR_LOCK_DATA **end_data2; DBUG_ENTER("mysql_lock_have_duplicate"); - /* Table may not be defined for derived or view tables. */ - if (! (table= needle->table)) + /* + Table may not be defined for derived or view tables. + Table may not be part of a lock for delayed operations. + */ + if (! (table= needle->table) || ! table->lock_count) goto end; /* A temporary table does not have locks. */ @@ -550,7 +553,8 @@ TABLE_LIST *mysql_lock_have_duplicate(THD *thd, TABLE_LIST *needle, lock_tables= mylock->table; /* Prepare table related variables that don't change in loop. */ - DBUG_ASSERT(table == lock_tables[table->lock_position]); + DBUG_ASSERT((table->lock_position < mylock->table_count) && + (table == lock_tables[table->lock_position])); table_lock_data= lock_locks + table->lock_data_start; end_data= table_lock_data + table->lock_count; @@ -563,7 +567,8 @@ TABLE_LIST *mysql_lock_have_duplicate(THD *thd, TABLE_LIST *needle, continue; /* All tables in list must be in lock. */ - DBUG_ASSERT(table2 == lock_tables[table2->lock_position]); + DBUG_ASSERT((table2->lock_position < mylock->table_count) && + (table2 == lock_tables[table2->lock_position])); for (lock_data2= lock_locks + table2->lock_data_start, end_data2= lock_data2 + table2->lock_count; diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index ad9606acb83..67aab57b880 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1536,7 +1536,10 @@ TABLE *delayed_insert::get_local_table(THD* client_thd) /* Adjust in_use for pointing to client thread */ copy->in_use= client_thd; - + + /* Adjust lock_count. This table object is not part of a lock. */ + copy->lock_count= 0; + return copy; /* Got fatal error */