Code review changes

sql/handler.cc

@@ -3658,6 +3658,8 @@ int handler::ha_write_row(uchar *buf)
   Write a record to the engine bypassing row-level binary logging.
   This method is used internally by the server for writing to
   performance schema tables, which are never replicated.
+  TODO: Merge this function with ha_write_row(), and provide a way
+  to disable the binlog there.
 */
 int handler::ha_write_row_no_binlog(uchar *buf)
 {

sql/lock.cc

@@ -111,21 +111,23 @@ static void print_lock_error(int error, const char *);
 static int thr_lock_errno_to_mysql[]=
 { 0, 1, ER_LOCK_WAIT_TIMEOUT, ER_LOCK_DEADLOCK };
 
-MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
-                              uint flags, bool *need_reopen)
+/**
+  Perform semantic checks for mysql_lock_tables.
+  @param thd The current thread
+  @param tables The tables to lock
+  @param count The number of tables to lock
+  @param flags Lock flags
+  @return 0 if all the check passed, non zero if a check failed.
+*/
+int mysql_lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags)
 {
-  MYSQL_LOCK *sql_lock;
-  TABLE *write_lock_used;
-  int rc;
-  uint i;
   bool log_table_write_query;
   uint system_count;
+  uint i;
 
-  DBUG_ENTER("mysql_lock_tables");
+  DBUG_ENTER("mysql_lock_tables_check");
 
-  *need_reopen= FALSE;
   system_count= 0;
-
   log_table_write_query= (is_log_table_write_query(thd->lex->sql_command)
                          || ((flags & MYSQL_LOCK_PERF_SCHEMA) != 0));
 
@@ -154,7 +156,7 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
           || (thd->lex->sql_command == SQLCOM_LOCK_TABLES))
       {
         my_error(ER_CANT_LOCK_LOG_TABLE, MYF(0));
-        DBUG_RETURN(0);
+        DBUG_RETURN(1);
       }
     }
 
@@ -173,9 +175,26 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
   if ((system_count > 0) && (system_count < count))
   {
     my_error(ER_WRONG_LOCK_OF_SYSTEM_TABLE, MYF(0));
-    DBUG_RETURN(0);
+    DBUG_RETURN(1);
   }
 
+  DBUG_RETURN(0);
+}
+
+MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
+                              uint flags, bool *need_reopen)
+{
+  MYSQL_LOCK *sql_lock;
+  TABLE *write_lock_used;
+  int rc;
+
+  DBUG_ENTER("mysql_lock_tables");
+
+  *need_reopen= FALSE;
+
+  if (mysql_lock_tables_check(thd, tables, count, flags))
+    DBUG_RETURN (NULL);
+
   for (;;)
   {
     if (! (sql_lock= get_lock_data(thd, tables, count, GET_LOCK_STORE_LOCKS,

sql/log.cc

@@ -708,19 +708,10 @@ bool Log_to_file_event_handler::
            longlong query_time, longlong lock_time, bool is_command,
            const char *sql_text, uint sql_text_len)
 {
-  bool res;
-
-  (void) pthread_mutex_lock(mysql_slow_log.get_log_lock());
-
-  /* TODO: MYSQL_QUERY_LOG::write is not thread-safe */
-  res= mysql_slow_log.write(thd, current_time, query_start_arg,
-                            user_host, user_host_len,
-                            query_time, lock_time, is_command,
-                            sql_text, sql_text_len);
-
-  (void) pthread_mutex_unlock(mysql_slow_log.get_log_lock());
-
-  return res;
+  return mysql_slow_log.write(thd, current_time, query_start_arg,
+                              user_host, user_host_len,
+                              query_time, lock_time, is_command,
+                              sql_text, sql_text_len);
 }
 
 
@@ -736,18 +727,9 @@ bool Log_to_file_event_handler::
               const char *sql_text, uint sql_text_len,
               CHARSET_INFO *client_cs)
 {
-  bool res;
-
-  (void) pthread_mutex_lock (mysql_log.get_log_lock());
-
-  /* TODO: MYSQL_QUERY_LOG::write is not thread-safe */
-  res= mysql_log.write(event_time, user_host, user_host_len,
-                       thread_id, command_type, command_type_len,
-                       sql_text, sql_text_len);
-
-  (void) pthread_mutex_unlock (mysql_log.get_log_lock());
-
-  return res;
+  return mysql_log.write(event_time, user_host, user_host_len,
+                         thread_id, command_type, command_type_len,
+                         sql_text, sql_text_len);
 }
 
 
@@ -1959,6 +1941,8 @@ bool MYSQL_QUERY_LOG::write(time_t event_time, const char *user_host,
   struct tm start;
   uint time_buff_len= 0;
 
+  (void) pthread_mutex_lock(&LOCK_log);
+
   /* Test if someone closed between the is_open test and lock */
   if (is_open())
   {
@@ -2003,6 +1987,7 @@ bool MYSQL_QUERY_LOG::write(time_t event_time, const char *user_host,
       goto err;
   }
 
+  (void) pthread_mutex_unlock(&LOCK_log);
   return FALSE;
 err:
 
@@ -2011,6 +1996,7 @@ err:
     write_error= 1;
     sql_print_error(ER(ER_ERROR_ON_WRITE), name, errno);
   }
+  (void) pthread_mutex_unlock(&LOCK_log);
   return TRUE;
 }
 
@@ -2053,8 +2039,13 @@ bool MYSQL_QUERY_LOG::write(THD *thd, time_t current_time,
   bool error= 0;
   DBUG_ENTER("MYSQL_QUERY_LOG::write");
 
+  (void) pthread_mutex_lock(&LOCK_log);
+
   if (!is_open())
+  {
+    (void) pthread_mutex_unlock(&LOCK_log);
     DBUG_RETURN(0);
+  }
 
   if (is_open())
   {						// Safety agains reopen
@@ -2158,6 +2149,7 @@ bool MYSQL_QUERY_LOG::write(THD *thd, time_t current_time,
       }
     }
   }
+  (void) pthread_mutex_unlock(&LOCK_log);
   DBUG_RETURN(error);
 }
 

sql/log.h

@@ -213,9 +213,6 @@ public:
                 WRITE_CACHE);
   }
 
-  /* TODO: fix MYSQL_LOG::write to be thread safe instead. */
-  inline pthread_mutex_t* get_log_lock() { return &LOCK_log; }
-
 private:
   time_t last_time;
 };

sql/sql_parse.cc

@@ -210,7 +210,7 @@ void init_update_queries(void)
   sql_command_flags[SQLCOM_CREATE_VIEW]=    CF_CHANGES_DATA;
   sql_command_flags[SQLCOM_DROP_VIEW]=      CF_CHANGES_DATA;
   sql_command_flags[SQLCOM_CREATE_EVENT]=   CF_CHANGES_DATA;
-  sql_command_flags[SQLCOM_ALTER_EVENT]=    CF_CHANGES_DATA | CF_WRITE_LOGS_COMMAND;
+  sql_command_flags[SQLCOM_ALTER_EVENT]=    CF_CHANGES_DATA;
   sql_command_flags[SQLCOM_DROP_EVENT]=     CF_CHANGES_DATA;
 
   sql_command_flags[SQLCOM_UPDATE]=	    CF_CHANGES_DATA | CF_HAS_ROW_COUNT;