Cleanups during review of new code

mysql-test/t/sp.test:
  Use --disable_parsing instead of comments
sql/lock.cc:
  Remove compiler warning
sql/mysqld.cc:
  Always send valid flag argument to reload_acl_and_cache()
sql/sp_cache.cc:
  Simple optimization
  Don't use mutex to read 'long' variable
  Indentation fixes
sql/sp_head.cc:
  Fix comments to use /* */
  Set proc_info to 0 after close_thread_tables()
sql/sql_base.cc:
  remove not needed test
sql/sql_parse.cc:
  Always send valid flag argument to reload_acl_and_cache()
  Fixed indentation
  Ensure we get an error if reset_master() fails.

mysql-test/t/sp.test

@@ -2468,25 +2468,26 @@ drop table t3|
 #
 # BUG#4318
 #
-#QQ Don't know if HANDLER commands can work with SPs, or at all...
-# 
-#create table t3 (s1 int)|
-#insert into t3 values (3), (4)|
-#
-#--disable_warnings
-#drop procedure if exists bug4318|
-#--enable_warnings
-#create procedure bug4318()
-#  handler t3 read next|
-#
-#handler t3 open|
-## Expect no results, as tables are closed, but there shouldn't be any errors
-#call bug4318()|
-#call bug4318()|
-#handler t3 close|
-#
-#drop procedure bug4318|
-#drop table t3|
+
+--disable_parsing Don't know if HANDLER commands can work with SPs, or at all..
+create table t3 (s1 int)|
+insert into t3 values (3), (4)|
+
+--disable_warnings
+drop procedure if exists bug4318|
+--enable_warnings
+create procedure bug4318()
+  handler t3 read next|
+
+handler t3 open|
+# Expect no results, as tables are closed, but there shouldn't be any errors
+call bug4318()|
+call bug4318()|
+handler t3 close|
+
+drop procedure bug4318|
+drop table t3|
+--enable_parsing
 
 #
 # BUG#4902: Stored procedure with SHOW WARNINGS leads to packet error
@@ -2834,26 +2835,27 @@ drop table t3|
 #
 # BUG#6022: Stored procedure shutdown problem with self-calling function.
 #
-# This part of test is disabled until we implement support for
-# recursive stored functions.
-#--disable_warnings
-#drop function if exists bug6022|
-#--enable_warnings
-#
-#--disable_warnings
-#drop function if exists bug6022|
-#--enable_warnings
-#create function bug6022(x int) returns int
-#begin
-#  if x < 0 then
-#    return 0;
-#  else
-#    return bug6022(x-1);
-#  end if;
-#end|
-#
-#select bug6022(5)|
-#drop function bug6022|
+
+--disable_parsing until we implement support for recursive stored functions.
+--disable_warnings
+drop function if exists bug6022|
+--enable_warnings
+
+--disable_warnings
+drop function if exists bug6022|
+--enable_warnings
+create function bug6022(x int) returns int
+begin
+  if x < 0 then
+    return 0;
+  else
+    return bug6022(x-1);
+  end if;
+end|
+
+select bug6022(5)|
+drop function bug6022|
+--enable_parsing
 
 #
 # BUG#6029: Stored procedure specific handlers should have priority
@@ -3760,27 +3762,28 @@ drop procedure if exists bug7088_1|
 drop procedure if exists bug7088_2|
 --enable_warnings
 
-# psergey: temporarily disabled until Bar fixes BUG#11986
-# create procedure bug6063()
-#   lbel: begin end|
-# call bug6063()|
-# # QQ Known bug: this will not show the label correctly.
-# show create procedure bug6063|
-# 
-# set character set utf8|
-# create procedure bug7088_1()
-#   label1: begin end label1|
-# create procedure bug7088_2()
-#   läbel1: begin end|
-# call bug7088_1()|
-# call bug7088_2()|
-# set character set default|
-# show create procedure bug7088_1|
-# show create procedure bug7088_2|
-# 
-# drop procedure bug6063|
-# drop procedure bug7088_1|
-# drop procedure bug7088_2|
+--disable_parsing temporarily disabled until Bar fixes BUG#11986
+create procedure bug6063()
+  lbel: begin end|
+call bug6063()|
+# QQ Known bug: this will not show the label correctly.
+show create procedure bug6063|
+
+set character set utf8|
+create procedure bug7088_1()
+  label1: begin end label1|
+create procedure bug7088_2()
+  läbel1: begin end|
+call bug7088_1()|
+call bug7088_2()|
+set character set default|
+show create procedure bug7088_1|
+show create procedure bug7088_2|
+
+drop procedure bug6063|
+drop procedure bug7088_1|
+drop procedure bug7088_2|
+--enable_parsing
 
 #
 # BUG#9565: "Wrong locking in stored procedure if a sub-sequent procedure

sql/lock.cc

@@ -849,10 +849,6 @@ static void print_lock_error(int error, const char *table)
   So in this exceptional case the COMMIT should not be blocked by the FLUSH
   TABLES WITH READ LOCK.
 
-  TODO in MySQL 5.x: make_global_read_lock_block_commit() should be
-  killable. Normally CPU does not spend a long time in this function (COMMITs
-  are quite fast), but it would still be nice.
-
 ****************************************************************************/
 
 volatile uint global_read_lock=0;
@@ -1003,7 +999,7 @@ bool make_global_read_lock_block_commit(THD *thd)
     pthread_cond_wait(&COND_refresh, &LOCK_global_read_lock);
   DBUG_EXECUTE_IF("make_global_read_lock_block_commit_loop",
                   protect_against_global_read_lock--;);
-  if (error= thd->killed)
+  if ((error= test(thd->killed)))
     global_read_lock_blocks_commit--; // undo what we did
   else
     thd->global_read_lock= MADE_GLOBAL_READ_LOCK_BLOCK_COMMIT;

sql/mysqld.cc

@@ -1908,7 +1908,8 @@ static void check_data_home(const char *path)
 static void sig_reload(int signo)
 {
  // Flush everything
-  reload_acl_and_cache((THD*) 0,REFRESH_LOG, (TABLE_LIST*) 0, NULL);
+  bool not_used;
+  reload_acl_and_cache((THD*) 0,REFRESH_LOG, (TABLE_LIST*) 0, &not_used);
   signal(signo, SIG_ACK);
 }
 
@@ -2267,12 +2268,13 @@ static void *signal_hand(void *arg __attribute__((unused)))
     case SIGHUP:
       if (!abort_loop)
       {
+        bool not_used;
 	mysql_print_status();		// Print some debug info
 	reload_acl_and_cache((THD*) 0,
 			     (REFRESH_LOG | REFRESH_TABLES | REFRESH_FAST |
 			      REFRESH_GRANT |
 			      REFRESH_THREADS | REFRESH_HOSTS),
-			     (TABLE_LIST*) 0, NULL); // Flush logs
+			     (TABLE_LIST*) 0, &not_used); // Flush logs
       }
       break;
 #ifdef USE_ONE_SIGNAL_HAND

sql/sp_cache.cc

@@ -22,7 +22,7 @@
 #include "sp_head.h"
 
 static pthread_mutex_t Cversion_lock;
-static ulong Cversion = 0;
+static ulong volatile Cversion = 0;
 
 void
 sp_cache_init()
@@ -30,6 +30,7 @@ sp_cache_init()
   pthread_mutex_init(&Cversion_lock, MY_MUTEX_INIT_FAST);
 }
 
+
 void
 sp_cache_clear(sp_cache **cp)
 {
@@ -42,33 +43,31 @@ sp_cache_clear(sp_cache **cp)
   }
 }
 
+
 void
 sp_cache_insert(sp_cache **cp, sp_head *sp)
 {
   sp_cache *c= *cp;
+  ulong v;
 
   if (! c)
-    c= new sp_cache();
-  if (c)
   {
-    ulong v;
-
-    pthread_mutex_lock(&Cversion_lock); // LOCK
-    v= Cversion;
-    pthread_mutex_unlock(&Cversion_lock); // UNLOCK
+    if (!(c= new sp_cache()))
+      return;                                   // End of memory error
+  }
 
-    if (c->version < v)
-    {
-      if (*cp)
-	c->remove_all();
-      c->version= v;
-    }
-    c->insert(sp);
-    if (*cp == NULL)
-      *cp= c;
+  v= Cversion;		/* No need to lock when reading long variable */
+  if (c->version < v)
+  {
+    if (*cp)
+      c->remove_all();
+    c->version= v;
   }
+  c->insert(sp);
+  *cp= c;                                       // Update *cp if it was NULL
 }
 
+
 sp_head *
 sp_cache_lookup(sp_cache **cp, sp_name *name)
 {
@@ -78,10 +77,8 @@ sp_cache_lookup(sp_cache **cp, sp_name *name)
   if (! c)
     return NULL;
 
-  pthread_mutex_lock(&Cversion_lock); // LOCK
-  v= Cversion;
-  pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-
+  
+  v= Cversion;		/* No need to lock when reading long variable */
   if (c->version < v)
   {
     c->remove_all();
@@ -91,6 +88,7 @@ sp_cache_lookup(sp_cache **cp, sp_name *name)
   return c->lookup(name->m_qname.str, name->m_qname.length);
 }
 
+
 bool
 sp_cache_remove(sp_cache **cp, sp_name *name)
 {
@@ -114,14 +112,14 @@ sp_cache_remove(sp_cache **cp, sp_name *name)
   return found;
 }
 
+
 void
 sp_cache_invalidate()
 {
-  pthread_mutex_lock(&Cversion_lock); // LOCK
-  Cversion++;
-  pthread_mutex_unlock(&Cversion_lock); // UNLOCK
+  thread_safe_increment(Cversion, &Cversion_lock); // UNLOCK
 }
 
+
 static byte *
 hash_get_key_for_sp_head(const byte *ptr, uint *plen,
 			       my_bool first)
@@ -132,6 +130,7 @@ hash_get_key_for_sp_head(const byte *ptr, uint *plen,
   return (byte*) sp->m_qname.str;
 }
 
+
 static void
 hash_free_sp_head(void *p)
 {
@@ -140,16 +139,19 @@ hash_free_sp_head(void *p)
   delete sp;
 }
 
+
 sp_cache::sp_cache()
 {
   init();
 }
 
+
 sp_cache::~sp_cache()
 {
   hash_free(&m_hashtable);
 }
 
+
 void
 sp_cache::init()
 {
@@ -158,6 +160,7 @@ sp_cache::init()
   version= 0;
 }
 
+
 void
 sp_cache::cleanup()
 {

sql/sql_base.cc

@@ -1919,8 +1919,7 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter)
           has added its base tables after itself, adjust the boundary pointer
           accordingly.
         */
-        if (query_tables_last_own &&
-            query_tables_last_own == &(tables->next_global) &&
+        if (query_tables_last_own == &(tables->next_global) &&
             tables->view->query_tables)
           query_tables_last_own= tables->view->query_tables_last;
         

sql/sql_parse.cc

@@ -1863,17 +1863,18 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
     }
 #endif
   case COM_REFRESH:
-    {
-      statistic_increment(thd->status_var.com_stat[SQLCOM_FLUSH],
-			  &LOCK_status);
-      ulong options= (ulong) (uchar) packet[0];
-      if (check_global_access(thd,RELOAD_ACL))
-	break;
-      mysql_log.write(thd,command,NullS);
-      if (!reload_acl_and_cache(thd, options, (TABLE_LIST*) 0, NULL))
-        send_ok(thd);
+  {
+    bool not_used;
+    statistic_increment(thd->status_var.com_stat[SQLCOM_FLUSH],
+                        &LOCK_status);
+    ulong options= (ulong) (uchar) packet[0];
+    if (check_global_access(thd,RELOAD_ACL))
       break;
-    }
+    mysql_log.write(thd,command,NullS);
+    if (!reload_acl_and_cache(thd, options, (TABLE_LIST*) 0, &not_used))
+      send_ok(thd);
+    break;
+  }
 #ifndef EMBEDDED_LIBRARY
   case COM_SHUTDOWN:
   {
@@ -3822,13 +3823,13 @@ end_with_restore_list:
     lex->no_write_to_binlog= 1;
   case SQLCOM_FLUSH:
   {
+    bool write_to_binlog;
     if (check_global_access(thd,RELOAD_ACL) || check_db_used(thd, all_tables))
       goto error;
     /*
       reload_acl_and_cache() will tell us if we are allowed to write to the
       binlog or not.
     */
-    bool write_to_binlog;
     if (!reload_acl_and_cache(thd, lex->type, first_table, &write_to_binlog))
     {
       /*
@@ -6383,13 +6384,13 @@ void add_join_natural(TABLE_LIST *a,TABLE_LIST *b)
     tables              Tables to flush (if any)
     write_to_binlog     Depending on 'options', it may be very bad to write the
                         query to the binlog (e.g. FLUSH SLAVE); this is a
-                        pointer where, if it is not NULL, reload_acl_and_cache()
-                        will put 0 if it thinks we really should not write to
-                        the binlog. Otherwise it will put 1.
+                        pointer where reload_acl_and_cache() will put 0 if
+                        it thinks we really should not write to the binlog.
+                        Otherwise it will put 1.
 
   RETURN
     0	 ok
-    !=0  error
+    !=0  error.  thd->killed or thd->net.report_error is set
 */
 
 bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
@@ -6475,10 +6476,10 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
       */
       tmp_write_to_binlog= 0;
       if (lock_global_read_lock(thd))
-	return 1;
+	return 1;                               // Killed
       result=close_cached_tables(thd,(options & REFRESH_FAST) ? 0 : 1,
                                  tables);
-      if (make_global_read_lock_block_commit(thd))
+      if (make_global_read_lock_block_commit(thd)) // Killed
       {
         /* Don't leave things in a half-locked state */
         unlock_global_read_lock(thd);
@@ -6500,7 +6501,10 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
   {
     tmp_write_to_binlog= 0;
     if (reset_master(thd))
+    {
       result=1;
+      thd->fatal_error();                       // Ensure client get error
+    }
   }
 #endif
 #ifdef OPENSSL
@@ -6522,8 +6526,7 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
 #endif
  if (options & REFRESH_USER_RESOURCES)
    reset_mqh((LEX_USER *) NULL);
- if (write_to_binlog)
-   *write_to_binlog= tmp_write_to_binlog;
+ *write_to_binlog= tmp_write_to_binlog;
  return result;
 }