Fix for bug 254 :

we now make a distinction between if the master is < 3.23.57, 3.23 && >=57, and 4.x
(before the 2 3.23 were one). This is because in 3.23.57 we have a way to distinguish between
a Start_log_event written at server startup and one written at FLUSH LOGS, so we
have a way to know if the slave must drop old temp tables or not.
Change: mi->old_format was bool, now it's enum (to handle 3 cases). However, functions
which had 'bool old_format' as an argument have their prototypes unchanged, because
the old old_format == 0 now corresponds to the enum value BINLOG_FORMAT_CURRENT which
is equal to 0, so boolean tests are left untouched. The only case were we use mi->old_format
as an enum instead of casting it implicitly to a bool, is in Start_log_event::exec_event,
where we want to distinguish between the 3 possible enum values.


sql/log_event.cc:
  Fix for bug 254 :
  we now make a distinction between if the master is < 3.23.57, 3.23 && >=57, and 4.x
  (before the 2 3.23 were one), to know if the slave must drop old temp tables or not.
sql/slave.cc:
  Fix for bug 254 : mi->old_format is now enum.
  An unrelated comment.
sql/slave.h:
  fix for bug 254 : mi->old_format is now enum.

3 files changed, 53 insertions, 16 deletions

diff --git a/sql/log_event.cc b/sql/log_event.cc
index 369ef940af2..ff968babcf0 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -2000,11 +2000,8 @@ int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli,
 
   IMPLEMENTATION
     - To handle the case where the master died without a stop event,
-      we clean up all temporary tables + locks that we got.
-      However, we don't clean temporary tables if the master was 3.23
-      (this is because a 3.23 master writes a Start_log_event at every
-      binlog rotation; if we were not careful we would remove temp tables
-      on the slave when FLUSH LOGS is issued on the master). 
+      we clean up all temporary tables that we got, if we are sure we
+      can (see below).
 
   TODO
     - Remove all active user locks
@@ -2015,18 +2012,37 @@ int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli,
 
 int Start_log_event::exec_event(struct st_relay_log_info* rli)
 {
-  if (!rli->mi->old_format)
-  {
+
+  switch (rli->mi->old_format) {
+  case BINLOG_FORMAT_CURRENT : 
     /* 
-       If 4.0 master, all temporary tables have been deleted on the master; 
-       if 3.23 master, this is far from sure.
+       This is 4.x, so a Start_log_event is only at master startup,
+       so we are sure the master has restarted and cleared his temp tables.
     */
     close_temporary_tables(thd);
+    cleanup_load_tmpdir();
+    break;
+  /* 
+     Now the older formats; in that case load_tmpdir is cleaned up by the I/O
+     thread.
+  */
+  case BINLOG_FORMAT_323_LESS_57 :
     /*
-      If we have old format, load_tmpdir is cleaned up by the I/O thread
+      Cannot distinguish a Start_log_event generated at master startup and
+      one generated by master FLUSH LOGS, so cannot be sure temp tables
+      have to be dropped. So do nothing.
     */
-    cleanup_load_tmpdir();
+    break;
+ case BINLOG_FORMAT_323_GEQ_57 : 
+    /* Can distinguish, based on the value of 'created' */
+    if (created) /* this was generated at master startup*/
+      close_temporary_tables(thd);
+    break;
+  default :
+    /* this case is impossible */
+    return 1;
   }
+
   return Log_event::exec_event(rli);
 }
 
diff --git a/sql/slave.cc b/sql/slave.cc
index bead4323b0c..cc0fa26027f 100644
--- a/sql/slave.cc
+++ b/sql/slave.cc
@@ -962,13 +962,19 @@ static int check_master_version(MYSQL* mysql, MASTER_INFO* mi)
 {
   const char* errmsg= 0;
   
+  /*
+    Note the following switch will bug when we have MySQL branch 30 ;)
+  */
   switch (*mysql->server_version) {
   case '3':
-    mi->old_format = 1;
+    mi->old_format = 
+      (strncmp(mysql->server_version, "3.23.57", 7) < 0) /* < .57 */ ?
+      BINLOG_FORMAT_323_LESS_57 : 
+      BINLOG_FORMAT_323_GEQ_57 ;
     break;
   case '4':
   case '5':
-    mi->old_format = 0;
+    mi->old_format = BINLOG_FORMAT_CURRENT;
     break;
   default:
     errmsg = "Master reported unrecognized MySQL version";
@@ -1204,6 +1210,16 @@ int init_relay_log_info(RELAY_LOG_INFO* rli, const char* info_fname)
      Read_Master_Log_Pos: 9744 -rw-rw----    1 guilhem  qq           8192 Jun  5 16:27 gbichot2-relay-bin.002
 
     See how 4 is less than 7811 and 8192 is less than 9744.
+
+    WARNING: this is risky because the slave can stay like this for a long time;
+    then if it has a power failure, master.info says the I/O thread has read
+    until 9744 while the relay-log contains only until 8192 (the in-memory part
+    from 8192 to 9744 has been lost), so the SQL slave thread will miss some
+    events, silently breaking replication.
+    Ideally we would like to flush master.info only when we know that the relay
+    log has no in-memory tail.
+    Note that the above problem may arise only when only the IO thread is
+    started, which is unlikely.
   */
 
   if (open_log(&rli->relay_log, glob_hostname, opt_relay_logname,
diff --git a/sql/slave.h b/sql/slave.h
index 8832302056d..66000f45e69 100644
--- a/sql/slave.h
+++ b/sql/slave.h
@@ -35,6 +35,11 @@ extern my_bool opt_log_slave_updates;
 extern ulonglong relay_log_space_limit;
 struct st_master_info;
 
+enum enum_binlog_formats {
+  BINLOG_FORMAT_CURRENT=0, /* 0 is important for easy 'if (mi->old_format)' */
+  BINLOG_FORMAT_323_LESS_57, 
+  BINLOG_FORMAT_323_GEQ_57 };
+
 /*
   TODO: this needs to be redone, but for now it does not matter since
   we do not have multi-master yet.
@@ -266,15 +271,15 @@ typedef struct st_master_info
   int events_till_abort;
 #endif
   bool inited;
-  bool old_format;			/* master binlog is in 3.23 format */
+  enum enum_binlog_formats old_format;			/* master binlog is in 3.23 format */
   volatile bool abort_slave, slave_running;
   volatile ulong slave_run_id;
   bool ignore_stop_event;
   
   
   st_master_info()
-    :fd(-1), io_thd(0), inited(0), old_format(0),abort_slave(0),
-     slave_running(0), slave_run_id(0)
+    :fd(-1), io_thd(0), inited(0), old_format(BINLOG_FORMAT_CURRENT),
+     abort_slave(0),slave_running(0), slave_run_id(0)
   {
     host[0] = 0; user[0] = 0; password[0] = 0;
     bzero(&file, sizeof(file));