List:Commits« Previous MessageNext Message »
From:ingo Date:January 10 2006 11:47am
Subject:bk commit into 4.0 tree (ingo:1.2168) BUG#5390
View as plain text  
Below is the list of changes that have just been committed into a local
4.0 repository of mydev. When mydev does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.2168 06/01/10 12:47:33 ingo@stripped +4 -0
  BUG#5390 - problems with merge tables
  Optimizations suggested by Monty.
  We assume that mysql_lock_have_duplicate() is called
  when all tables are locked. Then we have no need to
  call get_lock_data() again. We just work on the existing lock.

  sql/table.h
    1.48 06/01/10 12:47:30 ingo@stripped +6 -0
    BUG#5390 - problems with merge tables
    Additional elements of TABLE to remember positions
    in a lock. And to protect against unwanted changes.

  sql/lock.cc
    1.56 06/01/10 12:47:30 ingo@stripped +153 -61
    BUG#5390 - problems with merge tables
    Optimizations suggested by Monty.
    We assume that mysql_lock_have_duplicate() is called
    when all tables are locked. Then we have no need to
    call get_lock_data() again. We just work on the existing lock.

  mysql-test/t/lock.test
    1.9 06/01/10 12:47:30 ingo@stripped +18 -0
    BUG#5390 - problems with merge tables
    Additional tests

  mysql-test/r/lock.result
    1.11 06/01/10 12:47:30 ingo@stripped +14 -0
    BUG#5390 - problems with merge tables
    Additional test results

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	ingo
# Host:	chilla.local
# Root:	/home/mydev/mysql-4.0-bug5390

--- 1.55/sql/lock.cc	2005-11-29 19:17:37 +01:00
+++ 1.56/sql/lock.cc	2006-01-10 12:47:30 +01:00
@@ -77,6 +77,11 @@
 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);
+static void save_lock_positions(TABLE **table_ptr, uint count,
+                                bool do_restore);
+/* Values for save_lock_positions(...,do_restore): */
+#define SAVE_LOCK_POSITIONS     0
+#define RESTORE_LOCK_POSITIONS  1
 
 
 /*
@@ -234,8 +239,10 @@
 {
   MYSQL_LOCK *sql_lock;
   TABLE *write_lock_used;
+  save_lock_positions(table, count, SAVE_LOCK_POSITIONS);
   if ((sql_lock = get_lock_data(thd, table, count, 1, &write_lock_used)))
     mysql_unlock_tables(thd, sql_lock);
+  save_lock_positions(table, count, RESTORE_LOCK_POSITIONS);
 }
 
 
@@ -299,20 +306,53 @@
     {
       if (locked->table[i] == table)
       {
-	locked->table_count--;
+        uint  j, lmove, lcount;
+        TABLE *tbl;
+        DBUG_PRINT("lock_data", ("removing table: '%s'  pos: %u  table_pos: %u",
+                                 table->table_name, i, table->lock_position));
+        DBUG_ASSERT(table->lock_position == i);
+
+        /* Decrement table_count in advance, making below expressions easier. */
+        lcount= --locked->table_count;
+
+        /* The table has 'lmove' lock data elements in locked->locks. */
+        lmove= table->lock_data_end - table->lock_data_start;
+
+        /* Move down all table pointers above 'i'. */
 	bmove((char*) (locked->table+i),
 	      (char*) (locked->table+i+1),
-	      (locked->table_count-i)* sizeof(TABLE*));
+	      (lcount - i) * sizeof(TABLE*));
+
+        /* Move down all lock data pointers above 'table->lock_data_end-1'. */
+        bmove((char*) (locked->locks + table->lock_data_start),
+              (char*) (locked->locks + table->lock_data_end),
+              (locked->lock_count - table->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.
+          lock_data_start and lock_data_end are the range of lock data
+          pointers for this table in the 'locked->locks' array, they
+          must be fixed by 'lmove', the lock data count of the removed table.
+        */
+        for (j= i ; j < lcount; j++)
+        {
+          tbl= locked->table[j];
+          tbl->lock_position--;
+          DBUG_ASSERT(tbl->lock_position == j);
+          DBUG_PRINT("lock_data", ("moving table: '%s'  to pos: %u",
+                                   tbl->table_name, j));
+          tbl->lock_data_end-= lmove;
+          tbl->lock_data_start-= lmove;
+        }
+
+        /* Finally adjust lock_count. */
+        locked->lock_count-= lmove;
 	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,12 +362,14 @@
 {
   MYSQL_LOCK *locked;
   TABLE *write_lock_used;
+  save_lock_positions(&table, 1, SAVE_LOCK_POSITIONS);
   if ((locked = get_lock_data(thd,&table,1,1,&write_lock_used)))
   {
     for (uint i=0; i < locked->lock_count; i++)
       thr_abort_locks(locked->locks[i]->lock);
     my_free((gptr) locked,MYF(0));
   }
+  save_lock_positions(&table, 1, RESTORE_LOCK_POSITIONS);
 }
 
 
@@ -351,6 +393,7 @@
   bool result= FALSE;
   DBUG_ENTER("mysql_lock_abort_for_thread");
 
+  save_lock_positions(&table, 1, SAVE_LOCK_POSITIONS);
   if ((locked = get_lock_data(thd,&table,1,1,&write_lock_used)))
   {
     for (uint i=0; i < locked->lock_count; i++)
@@ -361,6 +404,7 @@
     }
     my_free((gptr) locked,MYF(0));
   }
+  save_lock_positions(&table, 1, RESTORE_LOCK_POSITIONS);
   DBUG_RETURN(result);
 }
 
@@ -402,73 +446,70 @@
   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.
 
   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;
+  uint                  table_start;
+  MYSQL_LOCK            *mylock;
+  TABLE                 **lock_tables;
+  TABLE                 *table2;
+  THR_LOCK_DATA         **lock_locks;
+  THR_LOCK_DATA         **lock_data;
+  THR_LOCK_DATA         **end_data;
   THR_LOCK_DATA         **lock_data2;
   THR_LOCK_DATA         **end_data2;
-  THR_LOCK              *lock1;
+  THR_LOCK              *lock2;
   DBUG_ENTER("mysql_lock_have_duplicate");
 
-  if (! (sql_lock1= get_lock_data(thd, tables1, 1, 1, &write_lock_used)))
-    goto err0;
+  /* Get command lock or LOCK TABLES lock. */
+  mylock= thd->lock ? thd->lock : thd->locked_tables;
+  DBUG_ASSERT(mylock);
+
+  /* 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_start= table->lock_data_start;
+  end_data= lock_locks + table->lock_data_end;
+
+  for (; tables; tables= tables->next)
+  {
+    table2= tables->table;
+    /* All tables in list must be in lock. */
+    DBUG_ASSERT(table2 == lock_tables[table2->lock_position]);
 
-  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;
-
-  count= 1;
-  for (lock_data1= sql_lock1->locks,
-         end_data1= lock_data1 + sql_lock1->lock_count;
-       lock_data1 < end_data1;
-       lock_data1++)
-  {
-    lock1= (*lock_data1)->lock;
-    for (lock_data2= sql_lock2->locks,
-           end_data2= lock_data2 + sql_lock2->lock_count;
+    for (lock_data2=  lock_locks + table2->lock_data_start,
+           end_data2= lock_locks + table2->lock_data_end;
          lock_data2 < end_data2;
          lock_data2++)
     {
-      if ((*lock_data2)->lock == lock1)
-        goto end;
+      lock2= (*lock_data2)->lock;
+
+      for (lock_data= lock_locks + table_start;
+           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);
 }
 
 
@@ -506,8 +547,8 @@
 {
   uint i,tables,lock_count;
   MYSQL_LOCK *sql_lock;
-  THR_LOCK_DATA **locks;
-  TABLE **to;
+  THR_LOCK_DATA **locks, **locks_buf;
+  TABLE **to, **table_buf;
   DBUG_ENTER("get_lock_data");
 
   *write_lock_used=0;
@@ -525,8 +566,8 @@
 		  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;
 
@@ -535,6 +576,9 @@
     TABLE *table;
     if ((table=table_ptr[i])->tmp_table == TMP_TABLE)
       continue;
+    table->lock_position= to - table_buf;
+    DBUG_PRINT("lock_data", ("lock data table: '%s'  pos: %u",
+                             table->table_name, table->lock_position));
     *to++=table;
     enum thr_lock_type lock_type= table->reginfo.lock_type;
     if (lock_type >= TL_WRITE_ALLOW_WRITE)
@@ -547,11 +591,59 @@
 	DBUG_RETURN(0);
       }
     }
+    table->lock_data_start= locks - locks_buf;
     locks=table->file->store_lock(thd, locks, get_old_locks ? TL_IGNORE :
 				  lock_type);
+    table->lock_data_end= locks - locks_buf;
   }
   DBUG_RETURN(sql_lock);
 }
+
+
+/*
+  Save or restore lock positions of TABLEs.
+
+  SYNOPSIS
+    save_lock_positions()
+      table_ptr                 Array with TABLE pointers.
+      count                     Number of elements in array.
+      do_restore                If to restore instaed of save.
+
+  DESCRIPTION
+    In some situations get_lock_data() is called without the intention to
+    use the lock data for a lock. To avoid that the (existing) lock is
+    polluted, save the lock position, stored in TABLE before calling
+    get_lock_data() and restore them afterwards.
+
+  RETURN
+    void
+*/
+
+static void save_lock_positions(TABLE **table_ptr, uint count,
+                                bool do_restore)
+{
+  while (count--)
+  {
+    TABLE *table;
+
+    if ((table= *(table_ptr++))->tmp_table == TMP_TABLE)
+      continue;
+
+    if (do_restore)
+    {
+      table->lock_position=   table->saved_lock_position;
+      table->lock_data_end=   table->saved_lock_data_end;
+      table->lock_data_start= table->saved_lock_data_start;
+    }
+    else
+    {
+      table->saved_lock_position=   table->lock_position;
+      table->saved_lock_data_end=   table->lock_data_end;
+      table->saved_lock_data_start= table->lock_data_start;
+    }
+  }
+}
+
 
 /*****************************************************************************
 **  Lock table based on the name.

--- 1.47/sql/table.h	2004-12-11 13:51:41 +01:00
+++ 1.48/sql/table.h	2006-01-10 12:47:30 +01:00
@@ -86,6 +86,12 @@
   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 position in MYSQL_LOCK.locks */
+  uint lock_data_end;                   /* One behind last taken in above */
+  uint saved_lock_position;             /* safe for above */
+  uint saved_lock_data_start;           /* safe for above */
+  uint saved_lock_data_end;             /* safe for above */
   int current_lock;			/* Type of lock on table */
   enum tmp_table_type tmp_table;
   my_bool copy_blobs;			/* copy_blobs when storing */

--- 1.10/mysql-test/r/lock.result	2004-02-03 09:46:48 +01:00
+++ 1.11/mysql-test/r/lock.result	2006-01-10 12:47:30 +01:00
@@ -47,3 +47,17 @@
 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 t9 (c1 int);
+insert into t1 values (1);
+insert into t2 values (2);
+insert into t9 values (9);
+lock tables t9 read, t1 write, t1 as t7 read, t2 read, t1 as t8 read;
+drop table t9;
+insert into t1 select * from t2;
+select * from t1;
+c1
+1
+2
+drop table t1, t2;

--- 1.8/mysql-test/t/lock.test	2004-02-03 09:46:48 +01:00
+++ 1.9/mysql-test/t/lock.test	2006-01-10 12:47:30 +01:00
@@ -57,3 +57,21 @@
 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 later optimization
+# Check that a dropped table is correctly removed from a lock.
+#
+create table t1 (c1 int);
+create table t2 (c1 int);
+create table t9 (c1 int);
+insert into t1 values (1);
+insert into t2 values (2);
+insert into t9 values (9);
+lock tables t9 read, t1 write, t1 as t7 read, t2 read, t1 as t8 read;
+drop table t9;
+insert into t1 select * from t2;
+select * from t1;
+drop table t1, t2;
+
Thread
bk commit into 4.0 tree (ingo:1.2168) BUG#5390ingo10 Jan