bzr commit into mysql-6.0 branch (konstantin:2673) WL#3726

[Konstantin Osipov]

#At file:///opt/local/work/mysql-6.0-3726/

 2673 Konstantin Osipov	2008-06-25
      Fix a MyISAM-specific bug in the new implementation of 
      LOCK TABLES (WL#3726).
      If more than one instance of a MyISAM table are open in the 
      same connection, all of them must share the same status_param.
      Otherwise, unlock of a table may lead to lost records.
      See also comments in thr_lock.c.
modified:
  include/thr_lock.h
  mysql-test/r/lock.result
  mysql-test/t/lock.test
  mysys/thr_lock.c
  sql/lock.cc

per-file messages:
  include/thr_lock.h
    Declare thr_lock_merge_status()
  mysql-test/r/lock.result
    Update test results.
  mysql-test/t/lock.test
    Add a test case for the situation when the same table is locked
    twice by LOCK TABLES, and only one instance is updated.
  mysys/thr_lock.c
    Move the code that makes sure all status_params of the same table are
    shared into a separate function.
  sql/lock.cc
    Make sure that status_param is shared when a table is reopened.
=== modified file 'include/thr_lock.h'
--- a/include/thr_lock.h	2008-02-18 22:29:39 +0000
+++ b/include/thr_lock.h	2008-06-25 12:44:00 +0000
@@ -148,6 +148,8 @@ void thr_unlock(THR_LOCK_DATA *data);
 enum enum_thr_lock_result thr_multi_lock(THR_LOCK_DATA **data,
                                          uint count, THR_LOCK_OWNER *owner);
 void thr_multi_unlock(THR_LOCK_DATA **data,uint count);
+void
+thr_lock_merge_status(THR_LOCK_DATA **data, uint count);
 void thr_abort_locks(THR_LOCK *lock, my_bool upgrade_lock);
 my_bool thr_abort_locks_for_thread(THR_LOCK *lock, my_thread_id thread);
 void thr_print_locks(void);		/* For debugging */

=== modified file 'mysql-test/r/lock.result'
--- a/mysql-test/r/lock.result	2008-06-11 11:49:58 +0000
+++ b/mysql-test/r/lock.result	2008-06-25 12:44:00 +0000
@@ -217,5 +217,23 @@ a
 unlock tables;
 drop table t1, t2;
 #
+# Ensure that mi_copy_status is called for two instances
+# of the same table when it is reopened after a flush.
+# 
+drop table if exists t1;
+drop view if exists v1;
+create table t1 (c1 int);
+create view v1 as select * from t1;
+lock tables t1 write, v1 write;
+flush table t1;
+insert into t1 values (33);
+flush table t1;
+select * from t1;
+c1
+33
+unlock tables;
+drop table t1;
+drop view v1;
+#
 # End of 6.0 tests.
 #

=== modified file 'mysql-test/t/lock.test'
--- a/mysql-test/t/lock.test	2008-06-11 11:49:58 +0000
+++ b/mysql-test/t/lock.test	2008-06-25 12:44:00 +0000
@@ -261,6 +261,24 @@ select * from t1;
 unlock tables;
 drop table t1, t2;
 
+--echo #
+--echo # Ensure that mi_copy_status is called for two instances
+--echo # of the same table when it is reopened after a flush.
+--echo # 
+--disable_warnings
+drop table if exists t1;
+drop view if exists v1;
+--enable_warnings
+create table t1 (c1 int);
+create view v1 as select * from t1;
+lock tables t1 write, v1 write;
+flush table t1;
+insert into t1 values (33);
+flush table t1;
+select * from t1;
+unlock tables;
+drop table t1;
+drop view v1;
 
 --echo #
 --echo # End of 6.0 tests.

=== modified file 'mysys/thr_lock.c'
--- a/mysys/thr_lock.c	2008-04-09 01:07:00 +0000
+++ b/mysys/thr_lock.c	2008-06-25 12:44:00 +0000
@@ -981,12 +981,43 @@ thr_multi_lock(THR_LOCK_DATA **data, uin
 	   (long) pos[0]->lock, pos[0]->type); fflush(stdout);
 #endif
   }
-  /*
-    Ensure that all get_locks() have the same status
-    If we lock the same table multiple times, we must use the same
-    status_param!
-  */
+  thr_lock_merge_status(data, count);
+  DBUG_RETURN(THR_LOCK_SUCCESS);
+}
+
+
+/**
+  Ensure that all locks for a given table have the same
+  status_param.
+
+  This is a MyISAM and possibly Maria specific crutch. MyISAM
+  engine stores data file length, record count and other table
+  properties in status_param member of handler. When a table is
+  locked, connection-local copy is made from a global copy
+  (myisam_share) by mi_get_status(). When a table is unlocked,
+  the changed status is transferred back to the global share by
+  mi_update_status().
+
+  One thing MyISAM doesn't do is to ensure that when the same
+  table is opened twice in a connection all instances share the
+  same status_param. This is necessary, however: for one, to keep
+  all instances of a connection "on the same page" with regard to
+  the current state of the table. For other, unless this is done,
+  myisam_share will always get updated from the last unlocked
+  instance (in mi_update_status()), and when this instance was not
+  the one that was used to update data, records may be lost.
+
+  For each table, this function looks up the last lock_data in the
+  list of acquired locks, and makes sure that all other instances
+  share status_param with it.
+*/
+
+void
+thr_lock_merge_status(THR_LOCK_DATA **data, uint count)
+{
 #if !defined(DONT_USE_RW_LOCKS)
+  THR_LOCK_DATA **pos= data;
+  THR_LOCK_DATA **end= data + count;
   if (count > 1)
   {
     THR_LOCK_DATA *last_lock= end[-1];
@@ -1028,7 +1059,6 @@ thr_multi_lock(THR_LOCK_DATA **data, uin
     } while (pos != data);
   }
 #endif
-  DBUG_RETURN(THR_LOCK_SUCCESS);
 }
 
   /* free all locks */

=== modified file 'sql/lock.cc'
--- a/sql/lock.cc	2008-06-19 12:39:58 +0000
+++ b/sql/lock.cc	2008-06-25 12:44:00 +0000
@@ -672,6 +672,8 @@ MYSQL_LOCK *mysql_lock_merge(MYSQL_LOCK 
   /* Delete old, not needed locks */
   my_free((uchar*) a,MYF(0));
   my_free((uchar*) b,MYF(0));
+
+  thr_lock_merge_status(sql_lock->locks, sql_lock->lock_count);
   DBUG_RETURN(sql_lock);
 }