bk commit into 5.1 tree (sven:1.2650) BUG#32407

[Sven Sandberg]

Below is the list of changes that have just been committed into a local
5.1 repository of sven. When sven does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-11-28 19:13:44+01:00, sven@riska.(none) +3 -0
  BUG#32407: Impossible to do point-in-time recovery from older binlog
  
  Problem: it is unsafe to read base64-printed events without first
  reading the Format_description_log_event (FD).  Currently, mysqlbinlog
  cannot print the FD.
  
  I made FD able to print themselves, and made mysqlbinlog tell FD events
  to print themselves.

  client/mysqlbinlog.cc@stripped, 2007-11-28 19:13:43+01:00, sven@riska.(none) +77 -11
    
     - mysqlbinlog uses this and prints FD in base64, even if the user
       specified a --start-position that is after the FD.  The reason is
       that it is not safe to read other base64-printed events unless the
       FD is known.
    
     - Added a --no-base64-output flag to mysqlbinlog, which suppresses
       this printing of extra out-of-range base64 FD.  This can be used
       with binlogs that are known not to contain any base64 events.  If
       mysqlbinlog later finds an event that must be printed in base64, it
       generates an error.
    
    While I was here, I fixed some small things in the code:
    
     - replace hard-coded 4 by EVENT_TYPE_OFFSET in 3 places
    
     - The name of the logfile can be '-' to indicate stdin.  Before my
       patch, the code just checked if the first character is '-'; now it
       does a full strcmp().  Probably, all arguments that begin with a -
       are already handled somewhere else as flags, but I still think it
       is better that the code reflects what it is supposed to do, with as
       little dependencies as possible on other parts of the code.  If we
       one day implement that all command line arguments after -- are
       files (as most unix tools do), then we need this.

  sql/log_event.cc@stripped, 2007-11-28 19:13:43+01:00, sven@riska.(none) +9 -1
    Added base64 printing to Format_description_log_events.
    
    While I was here, I fixed two small issues:
    
     - Log_event::get_type_str() did not return anything for old row events.
    
     - DBUG_ENTER was not matched by DBUG_VOID_RETURN in one place.

  sql/log_event.h@stripped, 2007-11-28 19:13:43+01:00, sven@riska.(none) +5 -2
    Better comments.

diff -Nrup a/client/mysqlbinlog.cc b/client/mysqlbinlog.cc
--- a/client/mysqlbinlog.cc	2007-09-15 05:10:31 +02:00
+++ b/client/mysqlbinlog.cc	2007-11-28 19:13:43 +01:00
@@ -63,7 +63,7 @@ void sql_print_error(const char *format,
 
 static bool one_database=0, to_last_remote_log= 0, disable_log_bin= 0;
 static bool opt_hexdump= 0;
-static bool opt_base64_output= 0;
+static bool opt_base64_output= 0, opt_no_base64_output= 0;
 static const char* database= 0;
 static my_bool force_opt= 0, short_form= 0, remote_opt= 0;
 static my_bool debug_info_flag, debug_check_flag;
@@ -96,7 +96,14 @@ static my_bool file_not_closed_error= 0;
   This is because the event will be created (alloced) in read_log_event()
   (which returns a pointer) in check_header().
 */
-Format_description_log_event* glob_description_event; 
+static Format_description_log_event* glob_description_event;
+/*
+  This is set whenever a Format_description_event is printed.  Later,
+  when an event is printed in base64, this flag is tested: if no
+  Format_description_event has been seen, it is unsafe to print the
+  base64 event, so an error message is generated.
+*/
+static bool printed_fd_event= FALSE;
 
 static int dump_local_log_entries(PRINT_EVENT_INFO *print_event_info,
                                   const char* logname);
@@ -632,11 +639,19 @@ Create_file event for file_id: %u\n",exv
       break;
     }
     case FORMAT_DESCRIPTION_EVENT:
-      delete glob_description_event;
-      glob_description_event= (Format_description_log_event*) ev;
+      /*
+        glob_description_event == ev when this function is called from
+        check_header.
+      */
+      if (glob_description_event != ev)
+      {
+        delete glob_description_event;
+        glob_description_event= (Format_description_log_event*) ev;
+      }
       print_event_info->common_header_len=
         glob_description_event->common_header_len;
       ev->print(result_file, print_event_info);
+      printed_fd_event= TRUE;
       ev->temp_buf= 0; // as the event ref is zeroed
       /*
         We don't want this event to be deleted now, so let's hide it (I
@@ -678,6 +693,26 @@ Create_file event for file_id: %u\n",exv
 Begin_load_query event for file_id: %u\n", exlq->file_id);
       break;
     }
+    case TABLE_MAP_EVENT:
+    case WRITE_ROWS_EVENT:
+    case DELETE_ROWS_EVENT:
+    case UPDATE_ROWS_EVENT:
+    case PRE_GA_WRITE_ROWS_EVENT:
+    case PRE_GA_DELETE_ROWS_EVENT:
+    case PRE_GA_UPDATE_ROWS_EVENT:
+      if (!printed_fd_event)
+      {
+        if (opt_no_base64_output)
+          die("Error: --no-base64-output specified, but binlog contains a %s "
+              "event which cannot be printed without base64.",
+              ev->get_type_str());
+        else
+          die("Error: malformed binlog: it does not contain any "
+              "Format_description_log_event. I now found a %s event, which is "
+              "not safe to process without a Format_description_log_event.",
+              ev->get_type_str());
+      }
+      /* FALL THROUGH */
     default:
       ev->print(result_file, print_event_info);
     }
@@ -713,6 +748,11 @@ static struct my_option my_long_options[
    "should not be applied on production systems.",
    (uchar**) &opt_base64_output, (uchar**) &opt_base64_output, 0, GET_BOOL,
    NO_ARG, 0, 0, 0, 0, 0, 0},
+  {"no-base64-output", OPT_BASE64_OUTPUT,
+   "Do not print any binlog entries using base64 encoding. "
+   "If there are any row-based events, an error message will be printed.",
+   (uchar**) &opt_no_base64_output, (uchar**) &opt_no_base64_output, 0,
+   GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
   /*
     mysqlbinlog needs charsets knowledge, to be able to convert a charset
     number found in binlog to a charset name (to be able to print things
@@ -1305,8 +1345,21 @@ err:
 }
 
 
+/**
+  Reads the Format_description_log_event from the beginning of the
+  input file.
+
+  The Format_description_log_event is only read if it is outside the
+  range specified with --start-position; otherwise, it will be seen
+  later.  If this is an old binlog, a fake Format_description_event is
+  created.  This also prints a Format_description_log_event to the
+  output, unless we reach the --start-position range. In this case, it
+  is assumed that a Format_description_log_event will be found when
+  reading events the usual way.
+*/
 static void check_header(IO_CACHE* file, 
-                        Format_description_log_event **description_event) 
+                         Format_description_log_event **description_event,
+                         PRINT_EVENT_INFO *print_event_info)
 {
   uchar header[BIN_LOG_HEADER_SIZE];
   uchar buf[PROBE_HEADER_LEN];
@@ -1371,8 +1424,9 @@ Could not read entry at offset %lu : Err
     {
       DBUG_PRINT("info",("buf[4]=%d", buf[4]));
       /* always test for a Start_v3, even if no --start-position */
-      if (buf[4] == START_EVENT_V3)       /* This is 3.23 or 4.x */
+      if (buf[EVENT_TYPE_OFFSET] == START_EVENT_V3)
       {
+        /* This is 3.23 or 4.x */
         if (uint4korr(buf + EVENT_LEN_OFFSET) < 
             (LOG_EVENT_MINIMAL_HEADER_LEN + START_V3_HEADER_LEN))
         {
@@ -1384,8 +1438,9 @@ Could not read entry at offset %lu : Err
       }
       else if (tmp_pos >= start_position)
         break;
-      else if (buf[4] == FORMAT_DESCRIPTION_EVENT) /* This is 5.0 */
+      else if (buf[EVENT_TYPE_OFFSET] == FORMAT_DESCRIPTION_EVENT)
       {
+        /* This is 5.0 */
         Format_description_log_event *new_description_event;
         my_b_seek(file, tmp_pos); /* seek back to event's start */
         if (!(new_description_event= (Format_description_log_event*) 
@@ -1399,9 +1454,11 @@ at offset %lu ; this could be a log form
         }
         delete *description_event;
         *description_event= new_description_event;
+        if (!opt_no_base64_output)
+          process_event(print_event_info, *description_event, tmp_pos);
         DBUG_PRINT("info",("Setting description_event"));
       }
-      else if (buf[4] == ROTATE_EVENT)
+      else if (buf[EVENT_TYPE_OFFSET] == ROTATE_EVENT)
       {
         Log_event *ev;
         my_b_seek(file, tmp_pos); /* seek back to event's start */
@@ -1430,7 +1487,7 @@ static int dump_local_log_entries(PRINT_
   uchar tmp_buff[BIN_LOG_HEADER_SIZE];
   int error= 0;
 
-  if (logname && logname[0] != '-')
+  if (logname && strcmp(logname, "-") != 0)
   {
     if ((fd = my_open(logname, O_RDONLY | O_BINARY, MYF(MY_WME))) < 0)
       return 1;
@@ -1440,7 +1497,7 @@ static int dump_local_log_entries(PRINT_
       my_close(fd, MYF(MY_WME));
       return 1;
     }
-    check_header(file, &glob_description_event);
+    check_header(file, &glob_description_event, print_event_info);
   }
   else // reading from stdin;
   {
@@ -1462,7 +1519,7 @@ static int dump_local_log_entries(PRINT_
     if (init_io_cache(file, fileno(stdin), 0, READ_CACHE, (my_off_t) 0,
 		      0, MYF(MY_WME | MY_NABP | MY_DONT_CHECK_FILESIZE)))
       return 1;
-    check_header(file, &glob_description_event);
+    check_header(file, &glob_description_event, print_event_info);
     if (start_position)
     {
       /* skip 'start_position' characters from stdin */
@@ -1550,6 +1607,15 @@ int main(int argc, char** argv)
   if (!argc)
   {
     usage();
+    free_defaults(defaults_argv);
+    exit(1);
+  }
+
+  if (opt_base64_output && opt_no_base64_output)
+  {
+    usage();
+    printf("--base64-output and --no-base64-output cannot be used "
+           "together.\n");
     free_defaults(defaults_argv);
     exit(1);
   }
diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
--- a/sql/log_event.cc	2007-11-28 12:11:13 +01:00
+++ b/sql/log_event.cc	2007-11-28 19:13:43 +01:00
@@ -493,6 +493,9 @@ const char* Log_event::get_type_str()
   case WRITE_ROWS_EVENT: return "Write_rows";
   case UPDATE_ROWS_EVENT: return "Update_rows";
   case DELETE_ROWS_EVENT: return "Delete_rows";
+  case PRE_GA_WRITE_ROWS_EVENT: return "Write_rows_old";
+  case PRE_GA_UPDATE_ROWS_EVENT: return "Update_rows_old";
+  case PRE_GA_DELETE_ROWS_EVENT: return "Delete_rows_old";
   case BEGIN_LOAD_QUERY_EVENT: return "Begin_load_query";
   case EXECUTE_LOAD_QUERY_EVENT: return "Execute_load_query";
   case INCIDENT_EVENT: return "Incident";
@@ -2576,6 +2579,11 @@ void Start_log_event_v3::print(FILE* fil
 #else
     my_b_printf(&cache,"ROLLBACK%s\n", print_event_info->delimiter);
 #endif
+    if (temp_buf)
+    {
+      my_b_printf(&cache, "BINLOG '\n");
+      print_base64(&cache, print_event_info, FALSE);
+    }
   }
   DBUG_VOID_RETURN;
 }
@@ -5022,7 +5030,7 @@ Create_file_log_event::Create_file_log_e
                    Load_log_event::get_data_size() +
                    create_file_header_len + 1);
     if (len < block_offset)
-      return;
+      DBUG_VOID_RETURN;
     block = (char*)buf + block_offset;
     block_len = len - block_offset;
   }
diff -Nrup a/sql/log_event.h b/sql/log_event.h
--- a/sql/log_event.h	2007-10-30 09:51:08 +01:00
+++ b/sql/log_event.h	2007-11-28 19:13:43 +01:00
@@ -810,6 +810,10 @@ public:
   bool cache_stmt;
 
 #ifndef MYSQL_CLIENT
+  /*
+    this is ifndef'ed to avoid having to link mysqlbinlog against
+    libpthread
+  */
   THD* thd;
 
   Log_event();
@@ -852,9 +856,8 @@ public:
   {
     return thd ? thd->db : 0;
   }
-#else
+#else /* ifndef MYSQL_CLIENT */
   Log_event() : temp_buf(0) {}
-    /* avoid having to link mysqlbinlog against libpthread */
   static Log_event* read_log_event(IO_CACHE* file,
                                    const Format_description_log_event
                                    *description_event);