Slightly amended Monty's SP replication code cleanup patch

sql/item.cc

@@ -936,7 +936,7 @@ my_decimal *Item_name_const::val_decimal(my_decimal *decimal_value)
 {
   DBUG_ASSERT(fixed);
   my_decimal *val= value_item->val_decimal(decimal_value);
-  Item::null_value= value_item->null_value;
+  null_value= value_item->null_value;
   return val;
 }
 
@@ -944,7 +944,7 @@ my_decimal *Item_name_const::val_decimal(my_decimal *decimal_value)
 bool Item_name_const::is_null()
 {
   bool ret= value_item->is_null();
-  Item::null_value= value_item->null_value;
+  null_value= value_item->null_value;
   return ret;
 }
 

sql/item.h

@@ -853,7 +853,7 @@ public:
     return  value_item->save_in_field(field, no_conversions);
   }
 
-  inline bool send(Protocol *protocol, String *str)
+  bool send(Protocol *protocol, String *str)
   {
     return value_item->send(protocol, str);
   }

sql/log.cc

@@ -1579,6 +1579,10 @@ bool MYSQL_LOG::write(Log_event *event_info)
 
   if (thd->binlog_evt_union.do_union)
   {
+    /*
+      In Stored function; Remember that function call caused an update.
+      We will log the function call to the binary log on function exit
+    */
     thd->binlog_evt_union.unioned_events= TRUE;
     thd->binlog_evt_union.unioned_events_trans |= event_info->cache_stmt;
     DBUG_RETURN(0);

sql/sp_head.cc

@@ -672,7 +672,7 @@ int cmp_splocal_locations(Item_splocal * const *a, Item_splocal * const *b)
   variables with NAME_CONST('sp_var_name', value) calls.
  
   RETURN
-    0  Ok, thd->query{_length} either has been appropraiately replaced or
+    0  Ok, thd->query{_length} either has been appropriately replaced or
        there is no need for replacements.
     1  Out of memory error.
 */
@@ -683,6 +683,9 @@ static bool subst_spvars(THD *thd, sp_instr *instr, LEX_STRING *query_str)
   if (thd->prelocked_mode == NON_PRELOCKED && mysql_bin_log.is_open())
   {
     Dynamic_array<Item_splocal*> sp_vars_uses;
+    char *pbuf, *cur, buffer[512];
+    String qbuf(buffer, sizeof(buffer), &my_charset_bin);
+    int prev_pos, res;
 
     /* Find all instances of item_splocal used in this statement */
     for (Item *item= instr->free_list; item; item= item->next)
@@ -700,41 +703,35 @@ static bool subst_spvars(THD *thd, sp_instr *instr, LEX_STRING *query_str)
       Construct a statement string where SP local var refs are replaced
       with "NAME_CONST(name, value)"
     */
-    char buffer[512];
-    String qbuf(buffer, sizeof(buffer), &my_charset_bin);
     qbuf.length(0);
-    
-    char *cur= query_str->str;
-    int prev_pos= 0;
-    int res= 0;
+    cur= query_str->str;
+    prev_pos= res= 0;
     for (Item_splocal **splocal= sp_vars_uses.front(); 
          splocal < sp_vars_uses.back(); splocal++)
     {
+      Item *val;
       /* append the text between sp ref occurences */
-      res |= qbuf.append(cur + prev_pos, (*splocal)->pos_in_query - prev_pos);
+      res|= qbuf.append(cur + prev_pos, (*splocal)->pos_in_query - prev_pos);
       prev_pos= (*splocal)->pos_in_query + (*splocal)->m_name.length;
       
       /* append the spvar substitute */
-      res |= qbuf.append(" NAME_CONST('");
-      res |= qbuf.append((*splocal)->m_name.str, (*splocal)->m_name.length);
-      res |= qbuf.append("',");
-      Item *val= (*splocal)->this_item();
+      res|= qbuf.append(" NAME_CONST('");
+      res|= qbuf.append((*splocal)->m_name.str, (*splocal)->m_name.length);
+      res|= qbuf.append("',");
+      val= (*splocal)->this_item();
       DBUG_PRINT("info", ("print %p", val));
       val->print(&qbuf);
-      res |= qbuf.append(')');
+      res|= qbuf.append(')');
       if (res)
         break;
     }
-    res |= qbuf.append(cur + prev_pos, query_str->length - prev_pos);
+    res|= qbuf.append(cur + prev_pos, query_str->length - prev_pos);
     if (res)
       DBUG_RETURN(1);
 
-    char *pbuf= thd->alloc(qbuf.length()+1);
-
-    if (!pbuf)
+    if (!(pbuf= thd->strmake(qbuf.ptr(), qbuf.length())))
       DBUG_RETURN(1);
 
-    memcpy(pbuf, qbuf.ptr(), qbuf.length()+1);
     thd->query= pbuf;
     thd->query_length= qbuf.length();
   }
@@ -1054,9 +1051,9 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   if (need_binlog_call)
     mysql_bin_log.stop_union_events(thd);
 
-  if (thd->binlog_evt_union.unioned_events)
+  if (thd->binlog_evt_union.unioned_events && mysql_bin_log.is_open())
   {
-    char buf[64];
+    char buf[256];
     String bufstr(buf, sizeof(buf), &my_charset_bin);
     bufstr.length(0);
     bufstr.append("DO ", 3);
@@ -1070,17 +1067,14 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
     }
     bufstr.append(')');
     
-    if (mysql_bin_log.is_open())
-    {
-      bool transactional_table= FALSE;
     Query_log_event qinfo(thd, bufstr.ptr(), bufstr.length(),
                           thd->binlog_evt_union.unioned_events_trans, FALSE);
-      if (mysql_bin_log.write(&qinfo) && transactional_table)
+    if (mysql_bin_log.write(&qinfo) && 
+        thd->binlog_evt_union.unioned_events_trans)
     {
       push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_UNKNOWN_ERROR,
-                    "Invoked ROUTINE modified a transactional table but MYSQL"
-                    "failed to reflect this change in the binary log.");
-      }
+                   "Invoked ROUTINE modified a transactional table but MySQL "
+                   "failed to reflect this change in the binary log");
     }
   }
 
@@ -1901,15 +1895,18 @@ sp_instr_stmt::execute(THD *thd, uint *nextp)
 
   query= thd->query;
   query_length= thd->query_length;
-  if (!(res= alloc_query(thd, m_query.str, m_query.length+1)))
+  if (!(res= alloc_query(thd, m_query.str, m_query.length+1)) &&
+      !(res=subst_spvars(thd, this, &m_query)))
   {
+    /*
+      (the order of query cache and subst_spvars calls is irrelevant because
+      queries with SP vars can't be cached)
+    */
     if (query_cache_send_result_to_client(thd,
 					  thd->query, thd->query_length) <= 0)
     {
-      thd->query_str_binlog_unsuitable= subst_spvars(thd, this, &m_query);
       res= m_lex_keeper.reset_lex_and_exec_core(thd, nextp, FALSE, this);
       query_cache_end_of_result(thd);
-      thd->query_str_binlog_unsuitable= FALSE;
     }
     else
       *nextp= m_ip+1;

sql/sql_class.cc

@@ -178,7 +178,7 @@ THD::THD()
    rand_used(0), time_zone_used(0),
    last_insert_id_used(0), insert_id_used(0), clear_next_insert_id(0),
    in_lock_tables(0), bootstrap(0), derived_tables_processing(FALSE),
-   spcont(NULL), query_str_binlog_unsuitable(FALSE)
+   spcont(NULL)
 {
   current_arena= this;
   host= user= priv_user= db= ip= 0;

sql/sql_class.h

@@ -1350,13 +1350,6 @@ public:
     long    long_value;
   } sys_var_tmp;
   
-  /*
-    If true, thd->query is not a suitable query to write to binary log. This
-    is not handled everywhere currently - we check it only in statements
-    that can have SP variable references.
-  */
-  bool query_str_binlog_unsuitable;
-  
   struct {
     /* 
       If true, mysql_bin_log::write(Log_event) call will not write events to 

sql/sql_delete.cc

@@ -14,15 +14,12 @@
    along with this program; if not, write to the Free Software
    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA */
 
-
 /*
   Delete of records and truncate of tables.
 
   Multi-table deletes were introduced by Monty and Sinisa
 */
 
-
-
 #include "mysql_priv.h"
 #include "ha_innodb.h"
 #include "sql_select.h"
@@ -254,8 +251,7 @@ cleanup:
         thd->clear_error();
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_table, FALSE);
-      if ((thd->query_str_binlog_unsuitable || mysql_bin_log.write(&qinfo)) 
-           && transactional_table)
+      if (mysql_bin_log.write(&qinfo) && transactional_table)
 	error=1;
     }
     if (!transactional_table)
@@ -720,8 +716,7 @@ bool multi_delete::send_eof()
         thd->clear_error();
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_tables, FALSE);
-      if ((thd->query_str_binlog_unsuitable || mysql_bin_log.write(&qinfo))
-          && !normal_tables)
+      if (mysql_bin_log.write(&qinfo) && !normal_tables)
 	local_error=1;  // Log write failed: roll back the SQL statement
     }
     if (!transactional_tables)

sql/sql_insert.cc

@@ -592,8 +592,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
           thd->clear_error();
 	Query_log_event qinfo(thd, thd->query, thd->query_length,
 			      transactional_table, FALSE);
-	if ((thd->query_str_binlog_unsuitable ||
-            mysql_bin_log.write(&qinfo)) && transactional_table)
+	if (mysql_bin_log.write(&qinfo) && transactional_table)
 	  error=1;
       }
       if (!transactional_table)

sql/sql_parse.cc

@@ -620,7 +620,7 @@ void init_update_queries(void)
 bool is_update_query(enum enum_sql_command command)
 {
   DBUG_ASSERT(command >= 0 && command <= SQLCOM_END);
-  return uc_update_queries[command];
+  return uc_update_queries[command] != 0;
 }
 
 /*

sql/sql_update.cc

@@ -475,8 +475,7 @@ int mysql_update(THD *thd,
         thd->clear_error();
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_table, FALSE);
-      if ((thd->query_str_binlog_unsuitable || mysql_bin_log.write(&qinfo)) 
-          && transactional_table)
+      if (mysql_bin_log.write(&qinfo) && transactional_table)
 	error=1;				// Rollback update
     }
     if (!transactional_table)
@@ -1442,8 +1441,7 @@ bool multi_update::send_eof()
         thd->clear_error();
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_tables, FALSE);
-      if ((thd->query_str_binlog_unsuitable || mysql_bin_log.write(&qinfo))
-          && trans_safe)
+      if (mysql_bin_log.write(&qinfo) && trans_safe)
 	local_error= 1;				// Rollback update
     }
     if (!transactional_tables)