Merge bodhi.(none):/opt/local/work/mysql-5.0-runtime

into  bodhi.(none):/opt/local/work/mysql-5.1-runtime

mysql-test/include/mix1.inc

@@ -814,6 +814,39 @@ create table t1 (a int) engine=innodb;
 alter table t1 alter a set default 1;
 drop table t1;
 
+--echo
+--echo Bug#24918 drop table and lock / inconsistent between 
+--echo perm and temp tables
+--echo
+--echo Check transactional tables under LOCK TABLES
+--echo
+--disable_warnings
+drop table if exists t24918, t24918_tmp, t24918_trans, t24918_trans_tmp, 
+t24918_access;
+--enable_warnings
+create table t24918_access (id int);
+create table t24918 (id int) engine=myisam;
+create temporary table t24918_tmp (id int) engine=myisam;
+create table t24918_trans (id int) engine=innodb;
+create temporary table t24918_trans_tmp (id int) engine=innodb;
+
+lock table t24918 write, t24918_tmp write, t24918_trans write, t24918_trans_tmp write;
+drop table t24918;
+--error ER_TABLE_NOT_LOCKED
+select * from t24918_access;
+drop table t24918_trans;
+--error ER_TABLE_NOT_LOCKED
+select * from t24918_access;
+drop table t24918_trans_tmp;
+--error ER_TABLE_NOT_LOCKED
+select * from t24918_access;
+drop table t24918_tmp;
+--error ER_TABLE_NOT_LOCKED
+select * from t24918_access;
+unlock tables;
+
+drop table t24918_access;
+
 --echo End of 5.0 tests
 
 

mysql-test/r/innodb_mysql.result

@@ -814,6 +814,34 @@ drop table if exists t1;
 create table t1 (a int) engine=innodb;
 alter table t1 alter a set default 1;
 drop table t1;
+
+Bug#24918 drop table and lock / inconsistent between 
+perm and temp tables
+
+Check transactional tables under LOCK TABLES
+
+drop table if exists t24918, t24918_tmp, t24918_trans, t24918_trans_tmp, 
+t24918_access;
+create table t24918_access (id int);
+create table t24918 (id int) engine=myisam;
+create temporary table t24918_tmp (id int) engine=myisam;
+create table t24918_trans (id int) engine=innodb;
+create temporary table t24918_trans_tmp (id int) engine=innodb;
+lock table t24918 write, t24918_tmp write, t24918_trans write, t24918_trans_tmp write;
+drop table t24918;
+select * from t24918_access;
+ERROR HY000: Table 't24918_access' was not locked with LOCK TABLES
+drop table t24918_trans;
+select * from t24918_access;
+ERROR HY000: Table 't24918_access' was not locked with LOCK TABLES
+drop table t24918_trans_tmp;
+select * from t24918_access;
+ERROR HY000: Table 't24918_access' was not locked with LOCK TABLES
+drop table t24918_tmp;
+select * from t24918_access;
+ERROR HY000: Table 't24918_access' was not locked with LOCK TABLES
+unlock tables;
+drop table t24918_access;
 End of 5.0 tests
 CREATE TABLE `t2` (
 `k` int(11) NOT NULL auto_increment,

sql/handler.h

@@ -992,6 +992,7 @@ public:
   uint ref_length;
   FT_INFO *ft_handler;
   enum {NONE=0, INDEX, RND} inited;
+  bool locked;
   bool implicit_emptied;                /* Can be !=0 only if HEAP */
   const COND *pushed_cond;
   /*
@@ -1022,11 +1023,13 @@ public:
     estimation_rows_to_insert(0), ht(ht_arg),
     ref(0), key_used_on_scan(MAX_KEY), active_index(MAX_KEY),
     ref_length(sizeof(my_off_t)),
-    ft_handler(0), inited(NONE), implicit_emptied(0),
+    ft_handler(0), inited(NONE),
+    locked(FALSE), implicit_emptied(0),
     pushed_cond(0), next_insert_id(0), insert_id_for_cur_row(0)
     {}
   virtual ~handler(void)
   {
+    DBUG_ASSERT(locked == FALSE);
     /* TODO: DBUG_ASSERT(inited == NONE); */
   }
   virtual handler *clone(MEM_ROOT *mem_root);
@@ -1591,8 +1594,10 @@ public:
 
   /* lock_count() can be more than one if the table is a MERGE */
   virtual uint lock_count(void) const { return 1; }
-  /*
+  /**
-    NOTE that one can NOT rely on table->in_use in store_lock().  It may
+    Is not invoked for non-transactional temporary tables.
+
+    @note that one can NOT rely on table->in_use in store_lock().  It may
     refer to a different thread if called from mysql_lock_abort_for_thread().
   */
   virtual THR_LOCK_DATA **store_lock(THD *thd,
@@ -1723,6 +1728,29 @@ private:
     overridden by the storage engine class. To call these methods, use
     the corresponding 'ha_*' method above.
   */
+
+  /**
+    Is not invoked for non-transactional temporary tables.
+
+    Tells the storage engine that we intend to read or write data
+    from the table. This call is prefixed with a call to handler::store_lock()
+    and is invoked only for those handler instances that stored the lock.
+
+    Calls to rnd_init/index_init are prefixed with this call. When table
+    IO is complete, we call external_lock(F_UNLCK).
+    A storage engine writer should expect that each call to
+    ::external_lock(F_[RD|WR]LOCK is followed by a call to
+    ::external_lock(F_UNLCK). If it is not, it is a bug in MySQL.
+
+    The name and signature originate from the first implementation
+    in MyISAM, which would call fcntl to set/clear an advisory
+    lock on the data file in this method.
+
+    @param   lock_type    F_RDLCK, F_WRLCK, F_UNLCK
+
+    @return  non-0 in case of failure, 0 in case of success.
+    When lock_type is F_UNLCK, the return value is ignored.
+  */
   virtual int external_lock(THD *thd __attribute__((unused)),
                             int lock_type __attribute__((unused)))
   {

sql/lock.cc

@@ -244,7 +244,8 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
 
     thd->proc_info="System lock";
     DBUG_PRINT("info", ("thd->proc_info %s", thd->proc_info));
-    if (lock_external(thd, tables, count))
+    if (sql_lock->table_count && lock_external(thd, sql_lock->table,
+                                               sql_lock->table_count))
     {
       /* Clear the lock type of all lock data to avoid reusage. */
       reset_lock_data(sql_lock);
@@ -340,6 +341,7 @@ static int lock_external(THD *thd, TABLE **tables, uint count)
 	((*tables)->reginfo.lock_type >= TL_READ &&
 	 (*tables)->reginfo.lock_type <= TL_READ_NO_INSERT))
       lock_type=F_RDLCK;
+
     if ((error=(*tables)->file->ha_external_lock(thd,lock_type)))
     {
       print_lock_error(error, (*tables)->file->table_type());
@@ -447,10 +449,28 @@ void mysql_unlock_read_tables(THD *thd, MYSQL_LOCK *sql_lock)
 }
 
 
+/**
+  Try to find the table in the list of locked tables.
+  In case of success, unlock the table and remove it from this list.
 
-void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table)
+  @note This function has a legacy side effect: the table is
+  unlocked even if it is not found in the locked list.
+  It's not clear if this side effect is intentional or still
+  desirable. It might lead to unmatched calls to
+  unlock_external(). Moreover, a discrepancy can be left
+  unnoticed by the storage engine, because in
+  unlock_external() we call handler::external_lock(F_UNLCK) only
+  if table->current_lock is not F_UNLCK.
+
+  @param  always_unlock   specify explicitly if the legacy side
+                          effect is desired.
+*/
+
+void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table,
+                       bool always_unlock)
 {
-  mysql_unlock_some_tables(thd, &table,1);
+  if (always_unlock == TRUE)
+    mysql_unlock_some_tables(thd, &table, /* table count */ 1);
   if (locked)
   {
     reg1 uint i;
@@ -464,6 +484,10 @@ void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table)
 
         DBUG_ASSERT(table->lock_position == i);
 
+        /* Unlock if not yet unlocked */
+        if (always_unlock == FALSE)
+          mysql_unlock_some_tables(thd, &table, /* table count */ 1);
+
         /* Decrement table_count in advance, making below expressions easier */
         old_tables= --locked->table_count;
 

sql/mysql_priv.h

@@ -1907,7 +1907,8 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **table, uint count,
 void mysql_unlock_tables(THD *thd, MYSQL_LOCK *sql_lock);
 void mysql_unlock_read_tables(THD *thd, MYSQL_LOCK *sql_lock);
 void mysql_unlock_some_tables(THD *thd, TABLE **table,uint count);
-void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table);
+void mysql_lock_remove(THD *thd, MYSQL_LOCK *locked,TABLE *table,
+                       bool always_unlock);
 void mysql_lock_abort(THD *thd, TABLE *table, bool upgrade_lock);
 void mysql_lock_downgrade_write(THD *thd, TABLE *table,
                                 thr_lock_type new_lock_type);

sql/sql_base.cc

@@ -1677,17 +1677,42 @@ TABLE *find_temporary_table(THD *thd, TABLE_LIST *table_list)
 }
 
 
-/*
+/**
-  Close temporary table and unlink from thd->temporary tables
+  Drop a temporary table.
+
+  Try to locate the table in the list of thd->temporary_tables.
+  If the table is found:
+   - if the table is in thd->locked_tables, unlock it and
+     remove it from the list of locked tables. Currently only transactional
+     temporary tables are present in the locked_tables list.
+   - Close the temporary table, remove its .FRM
+   - remove the table from the list of temporary tables
+
+  This function is used to drop user temporary tables, as well as
+  internal tables created in CREATE TEMPORARY TABLE ... SELECT
+  or ALTER TABLE. Even though part of the work done by this function
+  is redundant when the table is internal, as long as we
+  link both internal and user temporary tables into the same
+  thd->temporary_tables list, it's impossible to tell here whether
+  we're dealing with an internal or a user temporary table.
+
+  @retval TRUE   the table was not found in the list of temporary tables
+                 of this thread
+  @retval FALSE  the table was found and dropped successfully.
 */
 
-bool close_temporary_table(THD *thd, TABLE_LIST *table_list)
+bool close_temporary_table(THD *thd, const char *db, const char *table_name)
 {
   TABLE *table;
 
   if (!(table= find_temporary_table(thd, table_list)))
     return 1;
   close_temporary_table(thd, table, 1, 1);
+  /*
+    If LOCK TABLES list is not empty and contains this table,
+    unlock the table and remove the table from this list.
+  */
+  mysql_lock_remove(thd, thd->locked_tables, table, FALSE);
   return 0;
 }
 
@@ -1833,7 +1858,7 @@ void unlink_open_table(THD *thd, TABLE *find, bool unlock)
 	!memcmp(list->s->table_cache_key.str, key, key_length))
     {
       if (unlock && thd->locked_tables)
-	mysql_lock_remove(thd, thd->locked_tables,list);
+        mysql_lock_remove(thd, thd->locked_tables, list, TRUE);
       VOID(hash_delete(&open_cache,(uchar*) list)); // Close table
     }
     else
@@ -1859,8 +1884,13 @@ void unlink_open_table(THD *thd, TABLE *find, bool unlock)
 
     @note This routine assumes that table to be closed is open only
           by calling thread so we needn't wait until other threads
-          will close the table. It also assumes that table to be
+          will close the table. Also unless called under implicit or
-          dropped is already unlocked.
+          explicit LOCK TABLES mode it assumes that table to be
+          dropped is already unlocked. In the former case it will
+          also remove lock on the table. But one should not rely on
+          this behaviour as it may change in future.
+          Currently, however, this function is never called for a
+          table that was locked with LOCK TABLES.
 */
 
 void drop_open_table(THD *thd, TABLE *table, const char *db_name,
@@ -2837,7 +2867,7 @@ void close_data_files_and_morph_locks(THD *thd, const char *db,
 	!strcmp(table->s->db.str, db))
     {
       if (thd->locked_tables)
-        mysql_lock_remove(thd, thd->locked_tables, table);
+        mysql_lock_remove(thd, thd->locked_tables, table, TRUE);
       table->open_placeholder= 1;
       close_handle_and_leave_table_as_lock(table);
     }
@@ -2975,7 +3005,7 @@ void close_old_data_files(THD *thd, TABLE *table, bool morph_locks,
             instances of this table.
           */
           mysql_lock_abort(thd, table, TRUE);
-          mysql_lock_remove(thd, thd->locked_tables, table);
+          mysql_lock_remove(thd, thd->locked_tables, table, TRUE);
           /*
             We want to protect the table from concurrent DDL operations
             (like RENAME TABLE) until we will re-open and re-lock it.
@@ -3120,7 +3150,7 @@ TABLE *drop_locked_tables(THD *thd,const char *db, const char *table_name)
     if (!strcmp(table->s->table_name.str, table_name) &&
 	!strcmp(table->s->db.str, db))
     {
-      mysql_lock_remove(thd, thd->locked_tables,table);
+      mysql_lock_remove(thd, thd->locked_tables, table, TRUE);
       if (!found)
       {
         found= table;

sql/sql_class.h

@@ -765,13 +765,25 @@ enum prelocked_mode_type {NON_PRELOCKED= 0, PRELOCKED= 1,
 class Open_tables_state
 {
 public:
-  /*
+  /**
-    open_tables - list of regular tables in use by this thread
+    List of regular tables in use by this thread. Contains temporary and
-    temporary_tables - list of temp tables in use by this thread
+    base tables that were opened with @see open_tables().
-    handler_tables - list of tables that were opened with HANDLER OPEN
-     and are still in use by this thread
   */
-  TABLE *open_tables, *temporary_tables, *handler_tables, *derived_tables;
+  TABLE *open_tables;
+  /**
+    List of temporary tables used by this thread. Contains user-level
+    temporary tables, created with CREATE TEMPORARY TABLE, and
+    internal temporary tables, created, e.g., to resolve a SELECT,
+    or for an intermediate table used in ALTER.
+    XXX Why are internal temporary tables added to this list?
+  */
+  TABLE *temporary_tables;
+  /**
+    List of tables that were opened with HANDLER OPEN and are
+    still in use by this thread.
+  */
+  TABLE *handler_tables;
+  TABLE *derived_tables;
   /*
     During a MySQL session, one can lock tables in two modes: automatic
     or manual. In automatic mode all necessary tables are locked just before