Merge spetrunia@bk-internal.mysql.com:/home/bk/mysql-5.0

into mysql.com:/home/psergey/mysql-5.0-bug12943

mysql-test/r/rpl_sp_effects.result

@@ -156,3 +156,60 @@ slave:	6
 drop procedure p1;
 drop function f1;
 drop table t1,t2;
+create table t1 (a int);
+create procedure p1()
+begin
+insert into t1 values(@x);
+set @x=@x+1;
+insert into t1 values(@x);
+if (f2()) then
+insert into t1 values(1243);
+end if;
+end//
+create function f2() returns int
+begin
+insert into t1 values(@z);
+set @z=@z+1;
+insert into t1 values(@z);
+return 0;
+end//
+create function f1() returns int
+begin
+insert into t1 values(@y);
+call p1();
+return 0;
+end//
+set @x=10;
+set @y=20;
+set @z=100;
+select f1();
+f1()
+0
+set @x=30;
+call p1();
+select 'master', a from t1;
+master	a
+master	20
+master	10
+master	11
+master	100
+master	101
+master	30
+master	31
+master	101
+master	102
+select 'slave', a from t1;
+slave	a
+slave	20
+slave	10
+slave	11
+slave	100
+slave	101
+slave	30
+slave	31
+slave	101
+slave	102
+drop table t1;
+drop function f1;
+drop function f2;
+drop procedure p1;

mysql-test/r/sp.result

@@ -3085,6 +3085,19 @@ column_name	bug10055(t.column_name)
 id	id
 data	data
 drop function bug10055|
+drop procedure if exists bug12297|
+create procedure bug12297(lim int)
+begin
+set @x = 0;
+repeat
+insert into t1(id,data)
+values('aa', @x);
+set @x = @x + 1;
+until @x >= lim
+end repeat;
+end|
+call bug12297(10)|
+drop procedure bug12297|
 drop function if exists f_bug11247|
 drop procedure if exists p_bug11247|
 create function f_bug11247(param int)

mysql-test/t/rpl_sp_effects.test

@@ -152,4 +152,52 @@ drop procedure p1;
 drop function f1;
 drop table t1,t2;
 
+# BUG#12637: User variables + SPs replication
+create table t1 (a int);
+delimiter //;
+create procedure p1()
+begin
+  insert into t1 values(@x);
+  set @x=@x+1;
+  insert into t1 values(@x);
+  if (f2()) then
+    insert into t1 values(1243);
+  end if;
+end//
+
+create function f2() returns int
+begin
+  insert into t1 values(@z);
+  set @z=@z+1;
+  insert into t1 values(@z);
+  return 0;
+end//
+
+create function f1() returns int
+begin
+  insert into t1 values(@y);
+  call p1();
+  return 0;
+end//
+
+delimiter ;//
+
+set @x=10;
+set @y=20;
+set @z=100;
+select f1();
+
+set @x=30;
+call p1();
+
+select 'master', a from t1;
+sync_slave_with_master;
+connection slave;
+select 'slave', a from t1;
+
+connection master;
+drop table t1;
+drop function f1;
+drop function f2;
+drop procedure p1;
 sync_slave_with_master;

mysql-test/t/sp.test

@@ -3877,29 +3877,23 @@ drop function bug10055|
 # consumption by passing large input parameter.
 #
 
-#
-#  Note: the test is currenly disabled because of the
-#  Bug #12637: SP crashes the server if it has update query with user var
-#  & binlog is enabled.
-#
-
 --disable_warnings
-#drop procedure if exists bug12297|
+drop procedure if exists bug12297|
 --enable_warnings
 
-#create procedure bug12297(lim int)
-#begin
-#  set @x = 0;
-#  repeat
-#   insert into t1(id,data)
-#   values('aa', @x);
-#   set @x = @x + 1;
-#  until @x >= lim
-#  end repeat;
-#end|
+create procedure bug12297(lim int)
+begin
+  set @x = 0;
+  repeat
+    insert into t1(id,data)
+    values('aa', @x);
+    set @x = @x + 1;
+  until @x >= lim
+  end repeat;
+end|
 
-#call bug12297(10)|
-#drop procedure bug12297|
+call bug12297(10)|
+drop procedure bug12297|
 
 #
 # Bug #11247 "Stored procedures: Function calls in long loops leak memory"

sql/item_func.cc

@@ -3884,7 +3884,8 @@ int get_var_with_binlog(THD *thd, enum_sql_command sql_command,
     if (!(var_entry= get_variable(&thd->user_vars, name, 0)))
       goto err;
   }
-  else if (var_entry->used_query_id == thd->query_id)
+  else if (var_entry->used_query_id == thd->query_id ||
+           mysql_bin_log.is_query_in_union(thd, var_entry->used_query_id))
   {
     /* 
        If this variable was already stored in user_var_events by this query
@@ -3901,10 +3902,16 @@ int get_var_with_binlog(THD *thd, enum_sql_command sql_command,
     appears:
     > set @a:=1;
     > insert into t1 values (@a), (@a:=@a+1), (@a:=@a+1);
-    We have to write to binlog value @a= 1;
+    We have to write to binlog value @a= 1.
+    
+    We allocate the user_var_event on user_var_events_alloc pool, not on
+    the this-statement-execution pool because in SPs user_var_event objects 
+    may need to be valid after current [SP] statement execution pool is
+    destroyed.
   */
   size= ALIGN_SIZE(sizeof(BINLOG_USER_VAR_EVENT)) + var_entry->length;
-  if (!(user_var_event= (BINLOG_USER_VAR_EVENT *) thd->alloc(size)))
+  if (!(user_var_event= (BINLOG_USER_VAR_EVENT *)
+        alloc_root(thd->user_var_events_alloc, size)))
     goto err;
   
   user_var_event->value= (char*) user_var_event +

sql/log.cc

@@ -1559,6 +1559,7 @@ void MYSQL_LOG::start_union_events(THD *thd)
   thd->binlog_evt_union.do_union= TRUE;
   thd->binlog_evt_union.unioned_events= FALSE;
   thd->binlog_evt_union.unioned_events_trans= FALSE;
+  thd->binlog_evt_union.first_query_id= thd->query_id;
 }
 
 void MYSQL_LOG::stop_union_events(THD *thd)
@@ -1567,6 +1568,12 @@ void MYSQL_LOG::stop_union_events(THD *thd)
   thd->binlog_evt_union.do_union= FALSE;
 }
 
+bool MYSQL_LOG::is_query_in_union(THD *thd, query_id_t query_id_param)
+{
+  return (thd->binlog_evt_union.do_union && 
+          query_id_param >= thd->binlog_evt_union.first_query_id);
+}
+
 /*
   Write an event to the binary log
 */

sql/sp_head.cc

@@ -689,10 +689,35 @@ int cmp_splocal_locations(Item_splocal * const *a, Item_splocal * const *b)
    * If this function invocation is done from a statement that is written
      into the binary log.
    * If there were any attempts to write events to the binary log during
-     function execution.
+     function execution (grep for start_union_events and stop_union_events)
+
    If the answers are No and Yes, we write the function call into the binary
    log as "DO spfunc(<param1value>, <param2value>, ...)"
   
+  
+  4. Miscellaneous issues.
+  
+  4.1 User variables. 
+
+  When we call mysql_bin_log.write() for an SP statement, thd->user_var_events
+  must hold set<{var_name, value}> pairs for all user variables used during 
+  the statement execution.
+  This set is produced by tracking user variable reads during statement
+  execution. 
+
+  Fo SPs, this has the following implications:
+  1) thd->user_var_events may contain events from several SP statements and 
+     needs to be valid after exection of these statements was finished. In 
+     order to achieve that, we
+     * Allocate user_var_events array elements on appropriate mem_root (grep
+       for user_var_events_alloc).
+     * Use is_query_in_union() to determine if user_var_event is created.
+     
+  2) We need to empty thd->user_var_events after we have wrote a function
+     call. This is currently done by making 
+     reset_dynamic(&thd->user_var_events);
+     calls in several different places. (TODO cosider moving this into
+     mysql_bin_log.write() function)
 */
 
 
@@ -908,6 +933,7 @@ int sp_head::execute(THD *thd)
     /* Don't change NOW() in FUNCTION or TRIGGER */
     if (!thd->in_sub_stmt)
       thd->set_time();		// Make current_time() et al work
+    
     /*
       We have to set thd->stmt_arena before executing the instruction
       to store in the instruction free_list all new items, created
@@ -915,6 +941,13 @@ int sp_head::execute(THD *thd)
       items made during other permanent subquery transformations).
     */
     thd->stmt_arena= i;
+    
+    /* will binlog this separately */
+    if (thd->prelocked_mode == NON_PRELOCKED) //TODO: change to event union?
+    {
+      thd->user_var_events_alloc= thd->mem_root;
+    }
+    
     ret= i->execute(thd, &ip);
 
     /*
@@ -929,15 +962,6 @@ int sp_head::execute(THD *thd)
 
     /* we should cleanup free_list and memroot, used by instruction */
     thd->free_items();
-    /*
-      FIXME: we must free user var events only if the routine is executed
-      in non-prelocked mode and statement-by-statement replication is used.
-      But if we don't free them now, the server crashes because user var
-      events are allocated in execute_mem_root. This is Bug#12637, and when
-      it's fixed, please add if (thd->options & OPTION_BIN_LOG) here.
-    */
-    if (opt_bin_log)
-      reset_dynamic(&thd->user_var_events);
     free_root(&execute_mem_root, MYF(0));
 
     /*
@@ -1095,7 +1119,10 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   binlog_save_options= thd->options;
   need_binlog_call= mysql_bin_log.is_open() && (thd->options & OPTION_BIN_LOG);
   if (need_binlog_call)
+  {
+    reset_dynamic(&thd->user_var_events);
     mysql_bin_log.start_union_events(thd);
+  }
     
   thd->options&= ~OPTION_BIN_LOG;
   ret= execute(thd);
@@ -1129,6 +1156,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
                    "Invoked ROUTINE modified a transactional table but MySQL "
                    "failed to reflect this change in the binary log");
     }
+    reset_dynamic(&thd->user_var_events);
   }
 
   if (m_type == TYPE_ENUM_FUNCTION && ret == 0)

sql/sp_head.h

@@ -108,13 +108,14 @@ class sp_head :private Query_arena
   MEM_ROOT main_mem_root;
 public:
   /* Possible values of m_flags */
-  const static int
+  enum {
     HAS_RETURN= 1,              // For FUNCTIONs only: is set if has RETURN
     IN_SIMPLE_CASE= 2,          // Is set if parsing a simple CASE
     IN_HANDLER= 4,              // Is set if the parser is in a handler body
     MULTI_RESULTS= 8,           // Is set if a procedure with SELECT(s)
     CONTAINS_DYNAMIC_SQL= 16,   // Is set if a procedure with PREPARE/EXECUTE
-    IS_INVOKED= 32;             // Is set if this sp_head is being used.
+    IS_INVOKED= 32              // Is set if this sp_head is being used.
+  };
 
   int m_type;			// TYPE_ENUM_FUNCTION or TYPE_ENUM_PROCEDURE
   uint m_flags;                 // Boolean attributes of a stored routine

sql/sql_class.h

@@ -313,6 +313,7 @@ class MYSQL_LOG: public TC_LOG
 
   void start_union_events(THD *thd);
   void stop_union_events(THD *thd);
+  bool is_query_in_union(THD *thd, query_id_t query_id_param);
 
   /*
     v stands for vector
@@ -1303,8 +1304,9 @@ class THD :public Statement,
   /* variables.transaction_isolation is reset to this after each commit */
   enum_tx_isolation session_tx_isolation;
   enum_check_fields count_cuted_fields;
-  /* for user variables replication*/
-  DYNAMIC_ARRAY user_var_events;
+
+  DYNAMIC_ARRAY user_var_events;        /* For user variables replication */
+  MEM_ROOT      *user_var_events_alloc; /* Allocate above array elements here */
 
   enum killed_state { NOT_KILLED=0, KILL_BAD_DATA=1, KILL_CONNECTION=ER_SERVER_SHUTDOWN, KILL_QUERY=ER_QUERY_INTERRUPTED };
   killed_state volatile killed;
@@ -1366,6 +1368,12 @@ class THD :public Statement,
       mysql_bin_log.start_union_events() call.
     */
     bool unioned_events_trans;
+    
+    /* 
+      'queries' (actually SP statements) that run under inside this binlog
+      union have thd->query_id >= first_query_id.
+    */
+    query_id_t first_query_id;
   } binlog_evt_union;
   
   THD();

sql/sql_parse.cc

@@ -5162,7 +5162,10 @@ void mysql_reset_thd_for_next_command(THD *thd)
   if (!thd->in_sub_stmt)
   {
     if (opt_bin_log)
+    {
       reset_dynamic(&thd->user_var_events);
+      thd->user_var_events_alloc= thd->mem_root;
+    }
     thd->clear_error();
     thd->total_warn_count=0;			// Warnings for this query
     thd->rand_used= 0;