Fix for BUG#12637: Make SPs+user variables replication work:

* Allocate thd->user_var_events elements on appropriate mem_root
* If several SP statements are binlogged as a single statement, collect all user var
  accesses they make (grep for StoredRoutinesBinlogging for details)


mysql-test/r/rpl_sp_effects.result:
  Testcase for BUG#12637
mysql-test/r/sp.result:
  re-enabled test case for BUG#12297
mysql-test/t/rpl_sp_effects.test:
  Testcase for BUG#12637
mysql-test/t/sp.test:
  re-enabled test case for BUG#12297
sql/item_func.cc:
  Fix for BUG#12637: Make SPs+user variables replication work:
  * Allocate thd->user_var_events elements on appropriate mem_root
  * If several SP statements are binlogged as a single statement, collect all user var
    accesses they make.
sql/log.cc:
  Fix for BUG#12637: Make SPs+user variables replication work:
  * Allocate thd->user_var_events elements on appropriate mem_root
  * If several SP statements are binlogged as a single statement, collect all user var
    accesses they make.
sql/sp_head.cc:
  Fix for BUG#12637: Make SPs+user variables replication work:
  * Allocate thd->user_var_events elements on appropriate mem_root
  * If several SP statements are binlogged as a single statement, collect all user var
    accesses they make.
sql/sp_head.h:
  Remove compiler warning.
sql/sql_class.h:
  Fix for BUG#12637: Make SPs+user variables replication work.
sql/sql_parse.cc:
  Fix for BUG#12637: Make SPs+user variables replication work.

6 files changed, 73 insertions, 19 deletions

diff --git a/sql/item_func.cc b/sql/item_func.cc
index 22d9fbbad34..c9565801389 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -3879,7 +3879,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
@@ -3896,10 +3897,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)))
+  size= ALIGN_SIZE(sizeof(BINLOG_USER_VAR_EVENT)) + var_entry->length;
+  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 +
diff --git a/sql/log.cc b/sql/log.cc
index d62fa52a165..920a3fcff42 100644
--- a/sql/log.cc
+++ b/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
 */
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 14956138cbf..833a9209360 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -678,10 +678,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)
 */
 
 
@@ -897,6 +922,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
@@ -904,6 +930,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);
 
     /*
@@ -918,15 +951,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));
 
     /*
@@ -1084,7 +1108,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);
@@ -1118,6 +1145,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)
diff --git a/sql/sp_head.h b/sql/sp_head.h
index 1c54b1a567d..271119ff2fb 100644
--- a/sql/sp_head.h
+++ b/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
diff --git a/sql/sql_class.h b/sql/sql_class.h
index b7073ecf5c5..a0c61944c6a 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -313,6 +313,7 @@ public:
 
   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 @@ public:
   /* 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 @@ public:
       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();
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index e3f9c0dc148..31582e7b2f4 100644
--- a/sql/sql_parse.cc
+++ b/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;