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

into bodhi.(none):/opt/local/work/mysql-5.1-runtime sql/mysql_priv.h: Auto merged sql/sql_class.h: Auto merged sql/sql_lex.cc: Auto merged mysql-test/include/mix1.inc: Manual merge. mysql-test/r/innodb_mysql.result: Manual merge. sql/handler.h: Manual merge. sql/lock.cc: Manual merge. sql/opt_range.cc: Manual merge. sql/sql_base.cc: Manual merge. sql/sql_table.cc: Manual merge.
2025-01-29 02:05:57 +01:00 · 2007-07-31 20:00:05 +04:00 · 2007-07-31 20:00:05 +04:00 · 4c11e05ad2
commit 4c11e05ad2
parent 1ac6e95483 eb6c85e755
7 changed files with 178 additions and 22 deletions
--- a/mysql-test/include/mix1.inc
+++ b/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


--- a/mysql-test/r/innodb_mysql.result
+++ b/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,
--- a/sql/handler.h
+++ b/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)))
  {
--- a/sql/lock.cc
+++ b/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;

--- a/sql/mysql_priv.h
+++ b/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);
--- a/sql/sql_base.cc
+++ b/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
-          dropped is already unlocked.
+          will close the table. Also unless called under implicit or
+          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;
--- a/sql/sql_class.h
+++ b/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
-    temporary_tables - list of temp tables in use by this thread
-    handler_tables - list of tables that were opened with HANDLER OPEN
-     and are still in use by this thread
+  /**
+    List of regular tables in use by this thread. Contains temporary and
+    base tables that were opened with @see open_tables().
  */
-  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