List:Commits« Previous MessageNext Message »
From:Petr Chardin Date:September 15 2006 11:21am
Subject:bk commit into 5.1 tree (petr:1.2307) BUG#21966
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of cps. When cps 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@stripped, 2006-09-15 13:21:14+04:00, petr@stripped +13 -0
  Fix for Bug #17544 "Cannot do atomic log rotate",
  Bug #21785 "Server crashes after rename of the log table" and
  Bug #21966 "Strange warnings on create like/repair of the log
              tables"
  
  According to the patch, from now on, one should use RENAME to
  perform a log table rotation (this should also be reflected in
  the manual).
  
  Here is a sample:
  
  use mysql;
  CREATE TABLE IF NOT EXISTS general_log2 LIKE general_log;
  RENAME TABLE general_log TO general_log_backup, general_log2 TO general_log;
  
  The rules for Rename of the log tables are following:
        IF   1. Log tables are enabled
        AND  2. Rename operates on the log table and nothing is being
                renamed to the log table.
        DO   3. Throw an error message.
        ELSE 4. Perform rename.
   
  The very RENAME query will go the the old (backup) table. This is
  consistent with the behavoiur we have with binlog ROTATE LOGS
  statement.
  
  Other problems, which are solved by the patch are:
  
  1) Now REPAIR of the log table is exclusive operation (as it should be), this
     also eliminates lock-related warnings. and
  2) CREATE LIKE TABLE now usese usual read lock on the source table rather
     then name lock, which is too restrictive. This way we get rid of another
     log table-related warning, which occured because of the above fact
     (as a side-effect, name lock resulted in a warning).

  mysql-test/r/log_tables.result@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +68 -0
    update result file

  mysql-test/t/log_tables.test@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +83 -0
    Add tests for the bugs

  sql/handler.cc@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +3 -2
    update comment

  sql/handler.h@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +5 -1
    update function to reflect changes in log tables
    locking logic.

  sql/lock.cc@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +2 -1
    Now we allow locking of the log tables for "privileged" threads
    Privileged thread must explicitly close and lock log tables. This
    is required for admin operations such as REPAIR.

  sql/log.cc@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +156 -84
    Changes to the file:
    1) Add checks for table schema. It's more important now,
       as we allow rename of the log tables. Since we should
       check for schema when writing to a log table.
       E.g. if one created a table with one-only comlumn and
       renamed it to general_log, the server should cope with
       it.
    2) refactor LOGGER::flush(), so that we can now use the same
       machinery as we use in FLUSH LOGS in other statements:
       whenever we have to perform  a serious operation on the log
       tables, we have to
       (a) wait for concurrent operations on the log tables (such 
       as selects) to stop (b) close logs (c) lock logger, so that
       nobody attempts to write to closed logs. Then perform an
       exclusive operation, d) reenable logs and e) unlock logger.
    3) Add a function to check if a given table is a log table.
    4) Add support for "privileged" thread
    5) merge is_[general/slow]_log_table_enabled() into one function.

  sql/log.h@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +40 -8
    1) add a new call close_n_lock_tables(). Now we use it instead of
       LOGGER::flush() in FLUSH LOGS implementation.
    2) add a prototype for the function to check if a given
       table is a log table;
    3) add privileged table flag to table logger
    4) merge is_[general/slow]_log_table_enabled()
       into one function.

  sql/mysql_priv.h@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +0 -4
    move log table defines to log.h

  sql/share/errmsg.txt@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +2 -0
    Add a new error message for rename of the log tables

  sql/sql_rename.cc@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +72 -1
    Traverse the list of tables in mysql_rename_tables
    to make sure that log tables are processed correctly
    (that is, according to the rules specified in the
    main CS comment)

  sql/sql_table.cc@stripped, 2006-09-15 13:21:10+04:00, petr@stripped +72 -23
    1) mysql_admin_table() should disable logs if it performs
       exclusive admin operation on a log table. This way we
       also eliminate warning on REPAIR of the log table.
    2) mysql_create_like_table should read-lock the source table
       instead getting name lock on it. Name lock is too restrictive
       in this case.

  storage/csv/ha_tina.cc@stripped, 2006-09-15 13:21:11+04:00, petr@stripped +2 -2
    update function to reflect changes in log tables
    locking logic.

  storage/myisam/ha_myisam.cc@stripped, 2006-09-15 13:21:11+04:00, petr@stripped +4 -4
    update function to reflect changes in log tables
    locking logic.

# 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:	petr
# Host:	owlet.local
# Root:	/home/cps/mysql/devel/5.1-rename-bug

--- 1.190/storage/myisam/ha_myisam.cc	2006-09-15 13:21:26 +04:00
+++ 1.191/storage/myisam/ha_myisam.cc	2006-09-15 13:21:26 +04:00
@@ -255,7 +255,7 @@ err:
 bool ha_myisam::check_if_locking_is_allowed(uint sql_command,
                                             ulong type, TABLE *table,
                                             uint count,
-                                            bool called_by_logger_thread)
+                                            bool called_by_privileged_thread)
 {
   /*
     To be able to open and lock for reading system tables like 'mysql.proc',
@@ -273,10 +273,10 @@ bool ha_myisam::check_if_locking_is_allo
 
   /*
     Deny locking of the log tables, which is incompatible with
-    concurrent insert. Unless called from a logger THD:
-    general_log_thd or slow_log_thd.
+    concurrent insert. Unless called from a logger THD (general_log_thd
+    or slow_log_thd) or by a privileged thread.
   */
-  if (!called_by_logger_thread)
+  if (!called_by_privileged_thread)
     return check_if_log_table_locking_is_allowed(sql_command, type, table);
 
   return TRUE;

--- 1.262/sql/handler.cc	2006-09-15 13:21:26 +04:00
+++ 1.263/sql/handler.cc	2006-09-15 13:21:26 +04:00
@@ -1427,8 +1427,9 @@ bool handler::check_if_log_table_locking
 {
   /*
     Deny locking of the log tables, which is incompatible with
-    concurrent insert. Unless called from a logger THD:
-    general_log_thd or slow_log_thd.
+    concurrent insert. The routine is not called if the table is
+    being locked from a logger THD (general_log_thd or slow_log_thd)
+    or from a privileged thread (see log.cc for details)
   */
   if (table->s->log_table &&
       sql_command != SQLCOM_TRUNCATE &&

--- 1.235/sql/handler.h	2006-09-15 13:21:26 +04:00
+++ 1.236/sql/handler.h	2006-09-15 13:21:26 +04:00
@@ -957,6 +957,10 @@ public:
         thd     Handler of the thread, trying to lock the table
         table   Table handler to check
         count   Number of locks already granted to the table
+        called_by_privileged_thread TRUE if called from a logger THD
+                                    (general_log_thd or slow_log_thd)
+                                    or by a privileged thread, which
+                                    has the right to lock log tables.
 
     DESCRIPTION
       Check whether a handler allows to lock the table. For instance,
@@ -972,7 +976,7 @@ public:
   virtual bool check_if_locking_is_allowed(uint sql_command,
                                            ulong type, TABLE *table,
                                            uint count,
-                                           bool called_by_logger_thread)
+                                           bool called_by_privileged_thread)
   {
     return TRUE;
   }

--- 1.97/sql/lock.cc	2006-09-15 13:21:26 +04:00
+++ 1.98/sql/lock.cc	2006-09-15 13:21:26 +04:00
@@ -691,7 +691,8 @@ static MYSQL_LOCK *get_lock_data(THD *th
           check_if_locking_is_allowed(thd->lex->sql_command, thd->lex->type,
                                       table_ptr[i], count,
                                       (thd == logger.get_general_log_thd()) ||
-                                           (thd == logger.get_slow_log_thd())))
+                                      (thd == logger.get_slow_log_thd()) ||
+                                      (thd == logger.get_privileged_thread())))
       DBUG_RETURN(0);
   }
 

--- 1.223/sql/log.cc	2006-09-15 13:21:27 +04:00
+++ 1.224/sql/log.cc	2006-09-15 13:21:27 +04:00
@@ -92,6 +92,24 @@ struct binlog_trx_data {
 
 handlerton binlog_hton;
 
+/* Check if a given table is opened log table */
+int check_if_opened_log_table(uint db_len, const char *db,
+                              const char *table_name)
+{
+  if (db_len == 5 &&
+      !my_strcasecmp(system_charset_info, db, "mysql"))
+  {
+    if (!my_strcasecmp(system_charset_info, table_name, "general_log") &&
+        logger.is_log_table_enabled(QUERY_LOG_GENERAL))
+      return QUERY_LOG_GENERAL;
+    else
+      if (!my_strcasecmp(system_charset_info, table_name, "slow_log") &&
+          logger.is_log_table_enabled(QUERY_LOG_SLOW))
+        return QUERY_LOG_SLOW;
+  }
+  return 0;
+}
+
 /*
   Open log table of a given type (general or slow log)
 
@@ -192,6 +210,12 @@ bool Log_to_csv_event_handler::open_log_
     my_pthread_setspecific_ptr(THR_MALLOC, 0);
   }
 
+  /*
+    After a log table was opened, we should clear privileged thread
+    flag (which allows locking of a log table by a special thread, usually
+    the one who closed log tables temporarily).
+  */
+  privileged_thread= 0;
   DBUG_RETURN(error);
 }
 
@@ -208,6 +232,8 @@ Log_to_csv_event_handler::Log_to_csv_eve
   /* logger thread always works with mysql database */
   slow_log_thd->db= my_strdup("mysql", MYF(0));;
   slow_log_thd->db_length= 5;
+  /* no privileged thread exists at the moment */
+  privileged_thread= 0;
 }
 
 
@@ -314,9 +340,6 @@ bool Log_to_csv_event_handler::
     filled by the Logger (=> no need to load default ones).
   */
 
-  /* log table entries are not replicated at the moment */
-  tmp_disable_binlog(current_thd);
-
   /* Set current time. Required for CURRENT_TIMESTAMP to work */
   general_log_thd->start_time= event_time;
 
@@ -325,21 +348,36 @@ bool Log_to_csv_event_handler::
     default value (which is CURRENT_TIMESTAMP).
   */
 
-  table->field[1]->store(user_host, user_host_len, client_cs);
+  /* check that all columns exist */
+  if (!table->field[1] || !table->field[2] || !table->field[3] ||
+      !table->field[4] || !table->field[5])
+    goto err;
+
+  /* do a write */
+  if (table->field[1]->store(user_host, user_host_len, client_cs) ||
+      table->field[2]->store((longlong) thread_id, TRUE) ||
+      table->field[3]->store((longlong) server_id, TRUE) ||
+      table->field[4]->store(command_type, command_type_len, client_cs) ||
+      table->field[5]->store(sql_text, sql_text_len, client_cs))
+    goto err;
+
+  /* mark tables as not null */
   table->field[1]->set_notnull();
-  table->field[2]->store((longlong) thread_id, TRUE);
   table->field[2]->set_notnull();
-  table->field[3]->store((longlong) server_id, TRUE);
   table->field[3]->set_notnull();
-  table->field[4]->store(command_type, command_type_len, client_cs);
   table->field[4]->set_notnull();
-  table->field[5]->store(sql_text, sql_text_len, client_cs);
   table->field[5]->set_notnull();
+
+  /* log table entries are not replicated at the moment */
+  tmp_disable_binlog(current_thd);
+
   table->file->ha_write_row(table->record[0]);
 
   reenable_binlog(current_thd);
 
   return FALSE;
+err:
+  return TRUE;
 }
 
 
@@ -388,9 +426,6 @@ bool Log_to_csv_event_handler::
   if (unlikely(!logger.is_log_tables_initialized))
     return FALSE;
 
-  /* log table entries are not replicated at the moment */
-  tmp_disable_binlog(current_thd);
-
   /*
      Set start time for CURRENT_TIMESTAMP to the start of the query.
      This will be default value for the field[0]
@@ -403,19 +438,30 @@ bool Log_to_csv_event_handler::
     default value.
   */
 
+  if (!table->field[1] || !table->field[2] || !table->field[3] ||
+      !table->field[4] || !table->field[5] || !table->field[6] ||
+      !table->field[7] || !table->field[8] || !table->field[9] ||
+      !table->field[10])
+    goto err;
+
   /* store the value */
-  table->field[1]->store(user_host, user_host_len, client_cs);
+  if (table->field[1]->store(user_host, user_host_len, client_cs))
+    goto err;
 
   if (query_start_arg)
   {
     /* fill in query_time field */
-    table->field[2]->store(query_time, TRUE);
+    if (table->field[2]->store(query_time, TRUE))
+      goto err;
     /* lock_time */
-    table->field[3]->store(lock_time, TRUE);
+    if (table->field[3]->store(lock_time, TRUE))
+      goto err;
     /* rows_sent */
-    table->field[4]->store((longlong) thd->sent_row_count, TRUE);
+    if (table->field[4]->store((longlong) thd->sent_row_count, TRUE))
+      goto err;
     /* rows_examined */
-    table->field[5]->store((longlong) thd->examined_row_count, TRUE);
+    if (table->field[5]->store((longlong) thd->examined_row_count, TRUE))
+      goto err;
   }
   else
   {
@@ -428,14 +474,18 @@ bool Log_to_csv_event_handler::
   /* fill database field */
   if (thd->db)
   {
-    table->field[6]->store(thd->db, thd->db_length, client_cs);
+    if (table->field[6]->store(thd->db, thd->db_length, client_cs))
+      goto err;
     table->field[6]->set_notnull();
   }
 
   if (thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt)
   {
-    table->field[7]->store((longlong)
-                           thd->first_successful_insert_id_in_prev_stmt_for_binlog,
TRUE);
+    if (table->
+        field[7]->store((longlong)
+                        thd->first_successful_insert_id_in_prev_stmt_for_binlog,
+                        TRUE))
+      goto err;
     table->field[7]->set_notnull();
   }
 
@@ -447,16 +497,23 @@ bool Log_to_csv_event_handler::
   */
   if (thd->auto_inc_intervals_in_cur_stmt_for_binlog.nb_elements() > 0)
   {
-    table->field[8]->store((longlong)
-                           thd->auto_inc_intervals_in_cur_stmt_for_binlog.minimum(),
TRUE);
+    if (table->
+        field[8]->store((longlong)
+          thd->auto_inc_intervals_in_cur_stmt_for_binlog.minimum(), TRUE))
+      goto err;
     table->field[8]->set_notnull();
   }
 
-  table->field[9]->store((longlong) server_id, TRUE);
+  if (table->field[9]->store((longlong) server_id, TRUE))
+    goto err;
   table->field[9]->set_notnull();
 
   /* sql_text */
-  table->field[10]->store(sql_text,sql_text_len, client_cs);
+  if (table->field[10]->store(sql_text,sql_text_len, client_cs))
+    goto err;
+
+  /* log table entries are not replicated at the moment */
+  tmp_disable_binlog(current_thd);
 
   /* write the row */
   table->file->ha_write_row(table->record[0]);
@@ -464,6 +521,8 @@ bool Log_to_csv_event_handler::
   reenable_binlog(current_thd);
 
   DBUG_RETURN(0);
+err:
+  DBUG_RETURN(1);
 }
 
 bool Log_to_csv_event_handler::
@@ -652,61 +711,45 @@ bool LOGGER::reopen_log_table(uint log_t
   return table_log_handler->reopen_log_table(log_table_type);
 }
 
+void LOGGER::close_n_lock_tables(THD *thd)
+{
+  table_log_handler->close_n_lock_tables(thd);
+}
 
 bool LOGGER::flush_logs(THD *thd)
 {
-  TABLE_LIST close_slow_log, close_general_log;
-
-  /* reopen log tables */
-  bzero((char*) &close_slow_log, sizeof(TABLE_LIST));
-  close_slow_log.alias= close_slow_log.table_name=(char*) "slow_log";
-  close_slow_log.table_name_length= 8;
-  close_slow_log.db= (char*) "mysql";
-  close_slow_log.db_length= 5;
-
-  bzero((char*) &close_general_log, sizeof(TABLE_LIST));
-  close_general_log.alias= close_general_log.table_name=(char*) "general_log";
-  close_general_log.table_name_length= 11;
-  close_general_log.db= (char*) "mysql";
-  close_general_log.db_length= 5;
-
-  /* lock tables, in the case they are enabled */
-  if (logger.is_log_tables_initialized)
-  {
-    /*
-      This will lock and wait for all but the logger thread to release the
-      tables. Then we could reopen log tables. Then release the name locks.
-
-      NOTE: in fact, the first parameter used in lock_and_wait_for_table_name()
-      and table_log_handler->flush() could be any non-NULL THD, as the
-      underlying code makes certain assumptions about this.
-      Here we use one of the logger handler THD's. Simply because it
-      seems appropriate.
-    */
-    if (opt_slow_log)
-      lock_and_wait_for_table_name(table_log_handler->general_log_thd,
-				   &close_slow_log);
-    if (opt_log)
-      lock_and_wait_for_table_name(table_log_handler->general_log_thd,
-				   &close_general_log);
-  }
-
+  int rc= 0;
   /*
-    Deny others from logging to general and slow log,
-    while reopening tables.
+    Close log tables, in the case they are enabled and lock logger.
+    We always need to do this at the same time, as once the log tables
+    are closed, we cannot allow other threads to write anything to them.
+
+    In the case we don't use log tables, we still need to lock logger,
+    as we are going to flush log files, and again no concurrent writes
+    are allowed.
   */
-  logger.lock();
+  if (logger.is_log_tables_initialized)
+    table_log_handler->close_n_lock_tables(thd); // the locking happens here
+  else
+    logger.lock();
 
   /* reopen log files */
   file_log_handler->flush();
 
-  /* flush tables, in the case they are enabled */
+  /* reopen tables in the case they were enabled */
   if (logger.is_log_tables_initialized)
-    table_log_handler->flush(table_log_handler->general_log_thd,
-                             &close_slow_log, &close_general_log);
+  {
+    /*
+      we use | and not || here, to ensure that both reopen_log_table
+      are called, even if the first one fails
+    */
+    if ((opt_slow_log && table_log_handler->reopen_log_table(QUERY_LOG_SLOW))
|
+        (opt_log && table_log_handler->reopen_log_table(QUERY_LOG_GENERAL)))
+      rc= TRUE;
+  }
   /* end of log flush */
   logger.unlock();
-  return FALSE;
+  return rc;
 }
 
 
@@ -1014,31 +1057,61 @@ void LOGGER::deactivate_log_handler(THD 
 }
 
 
-bool Log_to_csv_event_handler::flush(THD *thd, TABLE_LIST *close_slow_log,
-                                     TABLE_LIST *close_general_log)
+void Log_to_csv_event_handler::close_n_lock_tables(THD *thd)
 {
+  TABLE_LIST close_slow_log, close_general_log;
+
+  /* fill lists, we will need to perform operations on tables */
+  bzero((char*) &close_slow_log, sizeof(TABLE_LIST));
+  close_slow_log.alias= close_slow_log.table_name=(char*) "slow_log";
+  close_slow_log.table_name_length= 8;
+  close_slow_log.db= (char*) "mysql";
+  close_slow_log.db_length= 5;
+
+  bzero((char*) &close_general_log, sizeof(TABLE_LIST));
+  close_general_log.alias= close_general_log.table_name=(char*) "general_log";
+  close_general_log.table_name_length= 11;
+  close_general_log.db= (char*) "mysql";
+  close_general_log.db_length= 5;
+
+  /*
+    This will lock and wait for all but the logger thread to release the
+    tables. Then we could close log tables and lock logger. Then release
+    the name locks.
+
+    NOTE: in fact, the first parameter used in lock_and_wait_for_table_name()
+    and table_log_handler->flush() could be any non-NULL THD, as the
+    underlying code makes certain assumptions about this.
+    Here we use one of the logger handler THD's. Simply because it
+    seems appropriate.
+  */
+  if (opt_slow_log)
+    lock_and_wait_for_table_name(general_log_thd, &close_slow_log);
+  if (opt_log)
+    lock_and_wait_for_table_name(general_log_thd, &close_general_log);
+
+  privileged_thread= thd;
+
+  /*
+    Now we lock logger, as nobody should be able to use logging routines while
+    log tables are closed
+  */
+  logger.lock();
+
   VOID(pthread_mutex_lock(&LOCK_open));
   if (opt_log)
   {
     close_log_table(QUERY_LOG_GENERAL, TRUE);
-    query_cache_invalidate3(thd, close_general_log, 0);
-    unlock_table_name(thd, close_general_log);
+    query_cache_invalidate3(general_log_thd, &close_general_log, 0);
+    unlock_table_name(general_log_thd, &close_general_log);
   }
   if (opt_slow_log)
   {
     close_log_table(QUERY_LOG_SLOW, TRUE);
-    query_cache_invalidate3(thd, close_slow_log, 0);
-    unlock_table_name(thd, close_slow_log);
+    query_cache_invalidate3(general_log_thd, &close_slow_log, 0);
+    unlock_table_name(general_log_thd, &close_slow_log);
   }
   VOID(pthread_mutex_unlock(&LOCK_open));
-  /*
-    we use | and not || here, to ensure that both reopen_log_table
-    are called, even if the first one fails
-  */
-  if ((opt_slow_log && reopen_log_table(QUERY_LOG_SLOW)) |
-      (opt_log && reopen_log_table(QUERY_LOG_GENERAL)))
-    return 1;
-  return 0;
 }
 
 /* the parameters are unused for the log tables */
@@ -1106,16 +1179,15 @@ void Log_to_csv_event_handler::
   THD *log_thd, *curr= current_thd;
   TABLE_LIST *table;
 
+  if (!logger.is_log_table_enabled(log_table_type))
+    return;                                     /* do nothing */
+
   switch (log_table_type) {
   case QUERY_LOG_GENERAL:
-    if (!logger.is_general_log_table_enabled())
-      return;                                     /* do nothing */
     log_thd= general_log_thd;
     table= &general_log;
     break;
   case QUERY_LOG_SLOW:
-    if (!logger.is_slow_log_table_enabled())
-      return;                                     /* do nothing */
     log_thd= slow_log_thd;
     table= &slow_log;
     break;

--- 1.433/sql/mysql_priv.h	2006-09-15 13:21:27 +04:00
+++ 1.434/sql/mysql_priv.h	2006-09-15 13:21:27 +04:00
@@ -1428,10 +1428,6 @@ void sql_print_information(const char *f
 typedef void (*sql_print_message_func)(const char *format, ...);
 extern sql_print_message_func sql_print_message_handlers[];
 
-/* type of the log table */
-#define QUERY_LOG_SLOW 1
-#define QUERY_LOG_GENERAL 2
-
 int error_log_print(enum loglevel level, const char *format,
                     va_list args);
 

--- 1.366/sql/sql_table.cc	2006-09-15 13:21:27 +04:00
+++ 1.367/sql/sql_table.cc	2006-09-15 13:21:27 +04:00
@@ -1624,9 +1624,9 @@ int mysql_rm_table_part2(THD *thd, TABLE
     if (share && share->log_table &&
         ((!my_strcasecmp(system_charset_info, table->table_name,
                          "general_log") && opt_log &&
-          logger.is_general_log_table_enabled()) ||
+          logger.is_log_table_enabled(QUERY_LOG_GENERAL)) ||
          (!my_strcasecmp(system_charset_info, table->table_name, "slow_log")
-          && opt_slow_log && logger.is_slow_log_table_enabled())))
+          && opt_slow_log &&
logger.is_log_table_enabled(QUERY_LOG_SLOW))))
     {
       my_error(ER_CANT_DROP_LOG_TABLE, MYF(0));
       DBUG_RETURN(1);
@@ -3993,7 +3993,6 @@ end:
 }
 
 
-
 /*
   RETURN VALUES
     FALSE Message sent to net (admin operation went ok)
@@ -4019,7 +4018,7 @@ static bool mysql_admin_table(THD* thd, 
   Item *item;
   Protocol *protocol= thd->protocol;
   LEX *lex= thd->lex;
-  int result_code;
+  int result_code, disable_logs= 0;
   DBUG_ENTER("mysql_admin_table");
 
   if (end_active_trans(thd))
@@ -4064,6 +4063,21 @@ static bool mysql_admin_table(THD* thd, 
     thd->no_warnings_for_error= no_warnings_for_error;
     if (view_operator_func == NULL)
       table->required_type=FRMTYPE_TABLE;
+
+    /*
+      If we want to perform an admin operation on the log table
+      (E.g. rename) and lock_type >= TL_READ_NO_INSERT disable
+      log tables
+    */
+
+    if (check_if_opened_log_table(table->db_length, table->db,
+                                  table->table_name) &&
+        lock_type >= TL_READ_NO_INSERT)
+    {
+      disable_logs= 1;
+      logger.close_n_lock_tables(thd);
+    }
+
     open_and_lock_tables(thd, table);
     thd->no_warnings_for_error= 0;
     table->next_global= save_next_global;
@@ -4380,11 +4394,27 @@ send_result_message:
   }
 
   send_eof(thd);
+  /* enable logging back if needed */
+  if (disable_logs)
+  {
+    if ((opt_slow_log && logger.reopen_log_table(QUERY_LOG_SLOW)) |
+        (opt_log && logger.reopen_log_table(QUERY_LOG_GENERAL)))
+       my_error(ER_CANT_ACTIVATE_LOG, MYF(0));
+    logger.unlock();
+  }
   DBUG_RETURN(FALSE);
 
  err:
   ha_autocommit_or_rollback(thd, 1);
   close_thread_tables(thd);			// Shouldn't be needed
+  /* enable logging back if needed */
+  if (disable_logs)
+  {
+    if ((opt_slow_log && logger.reopen_log_table(QUERY_LOG_SLOW)) |
+        (opt_log && logger.reopen_log_table(QUERY_LOG_GENERAL)))
+       my_error(ER_CANT_ACTIVATE_LOG, MYF(0));
+    logger.unlock();
+  }
   if (table)
     table->table=0;
   DBUG_RETURN(TRUE);
@@ -4549,6 +4579,7 @@ bool mysql_create_like_table(THD* thd, T
 {
   TABLE *tmp_table;
   char src_path[FN_REFLEN], dst_path[FN_REFLEN], tmp_path[FN_REFLEN];
+  char src_table_name_buff[FN_REFLEN], src_db_name_buff[FN_REFLEN];
   uint dst_path_length;
   char *db= table->db;
   char *table_name= table->table_name;
@@ -4585,13 +4616,6 @@ bool mysql_create_like_table(THD* thd, T
     DBUG_RETURN(-1);
   }
 
-  bzero((gptr)&src_tables_list, sizeof(src_tables_list));
-  src_tables_list.db= src_db;
-  src_tables_list.table_name= src_table;
-
-  if (lock_and_wait_for_table_name(thd, &src_tables_list))
-    goto err;
-
   if ((tmp_table= find_temporary_table(thd, src_db, src_table)))
     strxmov(src_path, tmp_table->s->path.str, reg_ext, NullS);
   else
@@ -4618,6 +4642,34 @@ bool mysql_create_like_table(THD* thd, T
     goto err;
   }
 
+  if (lower_case_table_names)
+  {
+    if (src_db)
+    {
+      strmake(src_db_name_buff, src_db,
+              min(sizeof(src_db_name_buff) - 1, table_ident->db.length));
+      my_casedn_str(files_charset_info, src_db_name_buff);
+      src_db= src_db_name_buff;
+    }
+    if (src_table)
+    {
+      strmake(src_table_name_buff, src_table,
+              min(sizeof(src_table_name_buff) - 1, table_ident->table.length));
+      my_casedn_str(files_charset_info, src_table_name_buff);
+      src_table= src_table_name_buff;
+    }
+  }
+
+  bzero((gptr)&src_tables_list, sizeof(src_tables_list));
+  src_tables_list.db= src_db;
+  src_tables_list.db_length= table_ident->db.length;
+  src_tables_list.lock_type= TL_READ;
+  src_tables_list.table_name= src_table;
+  src_tables_list.alias= src_table;
+
+  if (simple_open_n_lock_tables(thd, &src_tables_list))
+    DBUG_RETURN(TRUE);
+
   /*
     Validate the destination table
 
@@ -4764,9 +4816,7 @@ table_exists:
     my_error(ER_TABLE_EXISTS_ERROR, MYF(0), table_name);
 
 err:
-  pthread_mutex_lock(&LOCK_open);
-  unlock_table_name(thd, &src_tables_list);
-  pthread_mutex_unlock(&LOCK_open);
+  close_thread_tables(thd);
   DBUG_RETURN(res);
 }
 
@@ -5157,29 +5207,28 @@ bool mysql_alter_table(THD *thd,char *ne
       !my_strcasecmp(system_charset_info, table_list->db, "mysql") &&
       table_list->table_name)
   {
-    enum enum_table_kind { NOT_LOG_TABLE= 1, GENERAL_LOG, SLOW_LOG }
-         table_kind= NOT_LOG_TABLE;
+    int table_kind= 0;
 
     if (!my_strcasecmp(system_charset_info, table_list->table_name,
                        "general_log"))
-      table_kind= GENERAL_LOG;
+      table_kind= QUERY_LOG_GENERAL;
     else
       if (!my_strcasecmp(system_charset_info, table_list->table_name,
                          "slow_log"))
-        table_kind= SLOW_LOG;
+        table_kind= QUERY_LOG_SLOW;
 
     /* Disable alter of enabled log tables */
-    if ((table_kind == GENERAL_LOG && opt_log &&
-        logger.is_general_log_table_enabled()) ||
-       (table_kind == SLOW_LOG && opt_slow_log &&
-         logger.is_slow_log_table_enabled()))
+    if ((table_kind == QUERY_LOG_GENERAL && opt_log &&
+        logger.is_log_table_enabled(table_kind)) ||
+       (table_kind == QUERY_LOG_SLOW && opt_slow_log &&
+         logger.is_log_table_enabled(table_kind)))
     {
       my_error(ER_CANT_ALTER_LOG_TABLE, MYF(0));
       DBUG_RETURN(TRUE);
     }
 
     /* Disable alter of log tables to unsupported engine */
-    if ((table_kind == GENERAL_LOG || table_kind == SLOW_LOG) &&
+    if ((table_kind == QUERY_LOG_GENERAL || table_kind == QUERY_LOG_SLOW) &&
         (lex_create_info->used_fields & HA_CREATE_USED_ENGINE) &&
         (!lex_create_info->db_type || /* unknown engine */
         !(lex_create_info->db_type->db_type == DB_TYPE_MYISAM ||

--- 1.120/sql/share/errmsg.txt	2006-09-15 13:21:27 +04:00
+++ 1.121/sql/share/errmsg.txt	2006-09-15 13:21:27 +04:00
@@ -5960,3 +5960,5 @@ ER_HOSTNAME
 	eng "host name"
 ER_WRONG_STRING_LENGTH
 	eng "String '%-.70s' is too long for %s (should be no longer than %d)"
+ER_CANT_RENAME_LOG_TABLE
+        eng "Cannot rename a log table. Rename of the log table is allowed only for log
rotate and in this case you should also specify a table with appropriate schema to be
renamed back to the log table"

--- 1.58/storage/csv/ha_tina.cc	2006-09-15 13:21:27 +04:00
+++ 1.59/storage/csv/ha_tina.cc	2006-09-15 13:21:27 +04:00
@@ -817,9 +817,9 @@ void ha_tina::update_status()
 bool ha_tina::check_if_locking_is_allowed(uint sql_command,
                                           ulong type, TABLE *table,
                                           uint count,
-                                          bool called_by_logger_thread)
+                                          bool called_by_privileged_thread)
 {
-  if (!called_by_logger_thread)
+  if (!called_by_privileged_thread)
     return check_if_log_table_locking_is_allowed(sql_command, type, table);
 
   return TRUE;

--- 1.8/mysql-test/r/log_tables.result	2006-09-15 13:21:27 +04:00
+++ 1.9/mysql-test/r/log_tables.result	2006-09-15 13:21:27 +04:00
@@ -218,3 +218,71 @@ unlock tables;
 use mysql;
 lock tables general_log read local, help_category read local;
 unlock tables;
+use mysql;
+RENAME TABLE general_log TO renamed_general_log;
+ERROR HY000: Cannot rename a log table. Rename of the log table is allowed only for log
rotate and in this case you should also specify a table with appropriate schema to be
renamed back to the log table
+RENAME TABLE slow_log TO renamed_slow_log;
+ERROR HY000: Cannot rename a log table. Rename of the log table is allowed only for log
rotate and in this case you should also specify a table with appropriate schema to be
renamed back to the log table
+truncate table general_log;
+select * from general_log;
+event_time	user_host	thread_id	server_id	command_type	argument
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	select * from general_log
+truncate table slow_log;
+select * from slow_log;
+start_time	user_host	query_time	lock_time	rows_sent	rows_examined	db	last_insert_id	insert_id	server_id	sql_text
+create table general_log_new like general_log;
+rename table general_log TO renamed_general_log, general_log_new TO general_log;
+create table slow_log_new like slow_log;
+rename table slow_log TO renamed_slow_log, slow_log_new TO slow_log;
+rename table general_log TO general_log_new, renamed_general_log TO general_log, slow_log
to renamed_slow_log;
+ERROR HY000: Cannot rename a log table. Rename of the log table is allowed only for log
rotate and in this case you should also specify a table with appropriate schema to be
renamed back to the log table
+select * from general_log;
+event_time	user_host	thread_id	server_id	command_type	argument
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	create table slow_log_new like slow_log
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	rename table slow_log TO renamed_slow_log,
slow_log_new TO slow_log
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	rename table general_log TO general_log_new,
renamed_general_log TO general_log, slow_log to renamed_slow_log
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	select * from general_log
+select * from renamed_general_log;
+event_time	user_host	thread_id	server_id	command_type	argument
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	select * from general_log
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	truncate table slow_log
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	select * from slow_log
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	create table general_log_new like general_log
+TIMESTAMP	USER_HOST	THREAD_ID	1	Query	rename table general_log TO renamed_general_log,
general_log_new TO general_log
+select * from slow_log;
+start_time	user_host	query_time	lock_time	rows_sent	rows_examined	db	last_insert_id	insert_id	server_id	sql_text
+select * from renamed_slow_log;
+start_time	user_host	query_time	lock_time	rows_sent	rows_examined	db	last_insert_id	insert_id	server_id	sql_text
+set global general_log='OFF';
+RENAME TABLE general_log TO general_log2;
+set global slow_query_log='OFF';
+RENAME TABLE slow_log TO slow_log2;
+set global general_log='ON';
+ERROR HY000: Cannot activate 'general' log
+set global slow_query_log='ON';
+ERROR HY000: Cannot activate 'slow query' log
+RENAME TABLE general_log2 TO general_log;
+RENAME TABLE slow_log2 TO slow_log;
+set global general_log='ON';
+set global slow_query_log='ON';
+flush logs;
+flush logs;
+drop table renamed_general_log, renamed_slow_log;
+use test;
+use mysql;
+repair table general_log;
+Table	Op	Msg_type	Msg_text
+mysql.general_log	repair	status	OK
+repair table slow_log;
+Table	Op	Msg_type	Msg_text
+mysql.slow_log	repair	status	OK
+create table general_log_new like general_log;
+create table slow_log_new like slow_log;
+show tables like "%log%";
+Tables_in_mysql (%log%)
+general_log
+general_log_new
+slow_log
+slow_log_new
+drop table slow_log_new, general_log_new;
+use test;

--- 1.12/mysql-test/t/log_tables.test	2006-09-15 13:21:27 +04:00
+++ 1.13/mysql-test/t/log_tables.test	2006-09-15 13:21:27 +04:00
@@ -314,6 +314,89 @@ use mysql;
 lock tables general_log read local, help_category read local;
 unlock tables;
 
+#
+# Bug #17544 Cannot do atomic log rotate and
+# Bug #21785 Server crashes after rename of the log table
+#
+
+use mysql;
+# Should result in error
+--error ER_CANT_RENAME_LOG_TABLE
+RENAME TABLE general_log TO renamed_general_log;
+--error ER_CANT_RENAME_LOG_TABLE
+RENAME TABLE slow_log TO renamed_slow_log;
+
+#check rotate logs
+truncate table general_log;
+--replace_column 1 TIMESTAMP 2 USER_HOST 3 THREAD_ID
+select * from general_log;
+
+truncate table slow_log;
+--replace_column 1 TIMESTAMP 2 USER_HOST
+select * from slow_log;
+
+create table general_log_new like general_log;
+rename table general_log TO renamed_general_log, general_log_new TO general_log;
+
+create table slow_log_new like slow_log;
+rename table slow_log TO renamed_slow_log, slow_log_new TO slow_log;
+
+# check that rename checks more then first table in the list
+--error ER_CANT_RENAME_LOG_TABLE
+rename table general_log TO general_log_new, renamed_general_log TO general_log, slow_log
to renamed_slow_log;
+
+# now check the content of tables
+--replace_column 1 TIMESTAMP 2 USER_HOST 3 THREAD_ID
+select * from general_log;
+--replace_column 1 TIMESTAMP 2 USER_HOST 3 THREAD_ID
+select * from renamed_general_log;
+
+# the content of the slow log is empty, but we will try a select anyway
+--replace_column 1 TIMESTAMP 2 USER_HOST
+select * from slow_log;
+--replace_column 1 TIMESTAMP 2 USER_HOST
+select * from renamed_slow_log;
+
+# check that we can do whatever we want with disabled log
+set global general_log='OFF';
+RENAME TABLE general_log TO general_log2;
+
+set global slow_query_log='OFF';
+RENAME TABLE slow_log TO slow_log2;
+
+# this should fail
+--error ER_CANT_ACTIVATE_LOG
+set global general_log='ON';
+--error ER_CANT_ACTIVATE_LOG
+set global slow_query_log='ON';
+
+RENAME TABLE general_log2 TO general_log;
+RENAME TABLE slow_log2 TO slow_log;
+
+# this should work
+set global general_log='ON';
+set global slow_query_log='ON';
+# now check flush logs
+flush logs;
+flush logs;
+drop table renamed_general_log, renamed_slow_log;
+use test;
+
+#
+# Bug #21966 Strange warnings on repair of the log tables
+#
+
+use mysql;
+# check that no warning occurs on repair of the log tables
+repair table general_log;
+repair table slow_log;
+# check that no warning occurs on "create like" for the log tables
+create table general_log_new like general_log;
+create table slow_log_new like slow_log;
+show tables like "%log%";
+drop table slow_log_new, general_log_new;
+use test;
+
 # kill all connections
 disconnect con1;
 disconnect con2;

--- 1.13/sql/log.h	2006-09-15 13:21:27 +04:00
+++ 1.14/sql/log.h	2006-09-15 13:21:27 +04:00
@@ -403,6 +403,9 @@ public:
 };
 
 
+int check_if_opened_log_table(uint db_len, const char *db,
+                              const char *table_name);
+
 class Log_to_csv_event_handler: public Log_event_handler
 {
   /*
@@ -411,6 +414,16 @@ class Log_to_csv_event_handler: public L
     THD's of the query. The reason is the locking order and duration.
   */
   THD *general_log_thd, *slow_log_thd;
+  /*
+    This is for the thread, which called close_n_lock_tables. The thread
+    will be allowed to write-lock the log tables (as it explicitly disabled
+    logging). This is used for such operations as REPAIR, which require
+    exclusive lock on the log tables.
+    NOTE: there can be only one priviliged thread, as close_n_lock_tables
+    lockes logger exclusively, so no other thread could get privileged
+    status.
+   */
+  THD *privileged_thread;
   friend class LOGGER;
   TABLE_LIST general_log, slow_log;
 
@@ -435,13 +448,20 @@ public:
                            const char *command_type, uint command_type_len,
                            const char *sql_text, uint sql_text_len,
                             CHARSET_INFO *client_cs);
-  bool flush(THD *thd, TABLE_LIST *close_slow_Log,
-             TABLE_LIST* close_general_log);
+  void close_n_lock_tables(THD *thd);
   void close_log_table(uint log_type, bool lock_in_use);
   bool reopen_log_table(uint log_type);
+  THD* get_privileged_thread()
+  {
+    return privileged_thread;
+  }
 };
 
 
+/* type of the log table */
+#define QUERY_LOG_SLOW 1
+#define QUERY_LOG_GENERAL 2
+
 class Log_to_file_event_handler: public Log_event_handler
 {
   MYSQL_QUERY_LOG mysql_log;
@@ -497,13 +517,18 @@ public:
   {}
   void lock() { (void) pthread_mutex_lock(&LOCK_logger); }
   void unlock() { (void) pthread_mutex_unlock(&LOCK_logger); }
-  bool is_general_log_table_enabled()
-  {
-    return table_log_handler && table_log_handler->general_log.table != 0;
-  }
-  bool is_slow_log_table_enabled()
+  void close_n_lock_tables(THD *thd);
+  bool is_log_table_enabled(uint log_table_type)
   {
-    return table_log_handler && table_log_handler->slow_log.table != 0;
+    switch (log_table_type) {
+    case QUERY_LOG_SLOW:
+      return table_log_handler && table_log_handler->slow_log.table != 0;
+    case QUERY_LOG_GENERAL:
+      return table_log_handler && table_log_handler->general_log.table != 0;
+    default:
+      DBUG_ASSERT(0);
+      return FALSE;                             /* make compiler happy */
+    }
   }
   /*
     We want to initialize all log mutexes as soon as possible,
@@ -562,6 +587,13 @@ public:
     if (file_log_handler)
       return file_log_handler->get_mysql_log();
     return NULL;
+  }
+  THD* get_privileged_thread()
+  {
+    if (table_log_handler)
+      return table_log_handler->get_privileged_thread();
+    else
+      return NULL;
   }
 };
 

--- 1.37/sql/sql_rename.cc	2006-09-15 13:21:27 +04:00
+++ 1.38/sql/sql_rename.cc	2006-09-15 13:21:27 +04:00
@@ -35,7 +35,8 @@ static TABLE_LIST *reverse_table_list(TA
 bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list, bool silent)
 {
   bool error= 1;
-  TABLE_LIST *ren_table= 0;
+  TABLE_LIST *ren_table= 0, *new_table;
+  int disable_logs= 0;
   DBUG_ENTER("mysql_rename_tables");
 
   /*
@@ -52,6 +53,68 @@ bool mysql_rename_tables(THD *thd, TABLE
 
   if (wait_if_global_read_lock(thd,0,1))
     DBUG_RETURN(1);
+
+  if (logger.is_log_table_enabled(QUERY_LOG_GENERAL) ||
+      logger.is_log_table_enabled(QUERY_LOG_SLOW))
+  {
+
+    /*
+      Rules for rename of a log table:
+
+      IF   1. Log tables are enabled
+      AND  2. Rename operates on the log table and nothing is being
+              renamed to the log table.
+      DO   3. Throw an error message.
+      ELSE 4. Perform rename.
+    */
+
+    for (ren_table= table_list; ren_table; ren_table= new_table->next_local)
+    {
+      int log_table_rename= 0;
+      new_table= ren_table->next_local;
+
+      /*
+        If we found a log table, we need to make sure that there is another
+        table, which is renamed back to the log table.
+      */
+      if ((log_table_rename=
+           check_if_opened_log_table(ren_table->db_length,
+                                     ren_table->db, ren_table->table_name)))
+      {
+        TABLE_LIST *curr_table, *curr_new_table;
+
+        /*
+          Log table encoutered we will need to disable and lock logs
+          for duration of rename.
+        */
+        disable_logs= TRUE;
+
+        for (curr_table= new_table->next_local; curr_table;
+             curr_table= curr_new_table->next_local)
+        {
+          curr_new_table= curr_table->next_local;
+          if (log_table_rename ==
+              check_if_opened_log_table(curr_new_table->db_length,
+                                        curr_new_table->db,
+                                        curr_new_table->table_name))
+          {
+            log_table_rename= 0;
+            break;
+          }
+        }
+        if (log_table_rename)
+        {
+          my_error(ER_CANT_RENAME_LOG_TABLE, MYF(0));
+          DBUG_RETURN(1);
+        }
+      }
+    }
+
+    /* The function will close log tables and call logger.lock() */
+    if (disable_logs)
+      logger.close_n_lock_tables(thd);
+  }
+
   VOID(pthread_mutex_lock(&LOCK_open));
   if (lock_table_names(thd, table_list))
     goto err;
@@ -95,6 +158,14 @@ bool mysql_rename_tables(THD *thd, TABLE
 
 err:
   pthread_mutex_unlock(&LOCK_open);
+  /* enable logging back if needed */
+  if (disable_logs)
+  {
+    if ((opt_slow_log && logger.reopen_log_table(QUERY_LOG_SLOW)) |
+        (opt_log && logger.reopen_log_table(QUERY_LOG_GENERAL)))
+      error= TRUE;
+    logger.unlock();
+  }
   start_waiting_global_read_lock(thd);
   DBUG_RETURN(error);
 }
Thread
bk commit into 5.1 tree (petr:1.2307) BUG#21966Petr Chardin15 Sep