List:Commits« Previous MessageNext Message »
From:Andrei Elkin Date:January 30 2008 1:34pm
Subject:Re: bk commit into 5.1 tree (sven:1.2678) BUG#34141
View as plain text  
Sven,

thanks for addressing all issues!

The patch is okay to go.

regards,

Andrei

PS an info note is set further down the text.

> 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, 2008-01-30 14:12:40+01:00, sven@riska.(none) +6 -0
>   BUG#34141: mysqlbinlog cannot read 4.1 binlogs containing load data infile
>   Main problem: mysql 5.1 cannot read binlogs from 4.1.
>   Subproblem 1: There is a mistake in sql_ex_info::init. The read_str()
>   function updates its first argument to point to the next character to
>   read. However, it is applied only to a copy of the buffer pointer, so the
>   real buffer pointer is not updated.
>   Fix 1: do not take a copy of the buffer pointer. The copy was needed
>   because sql_ex_info::init does not use the const attribute on some of its
>   arguments. So we add the const attribute, too.
>   Subproblem 2: The first BINLOG statement is asserted to be a
>   FORMAT_DESCRIPTION_LOG_EVENT, but 4.1 binlogs begin with START_EVENT_V3.
>   Fix 2: allow START_EVENT_V3 too.
>
>   mysql-test/suite/binlog/r/binlog_old_versions.result@stripped, 2008-01-30
> 14:12:31+01:00, sven@riska.(none) +10 -0
>     Updated result file.
>
>   mysql-test/suite/binlog/std_data/binlog_old_version_4_1.000001@stripped, 2008-01-30
> 14:06:40+01:00, sven@riska.(none) +3322 -0
>     New BitKeeper file
> ``mysql-test/suite/binlog/std_data/binlog_old_version_4_1.000001''
>
>   mysql-test/suite/binlog/std_data/binlog_old_version_4_1.000001@stripped, 2008-01-30
> 14:06:40+01:00, sven@riska.(none) +0 -0
>
>   mysql-test/suite/binlog/t/binlog_old_versions.test@stripped, 2008-01-30 14:12:31+01:00,
> sven@riska.(none) +15 -0
>     Added a test reading an old 4.1 binlog.
>
>   sql/log_event.cc@stripped, 2008-01-30 14:12:32+01:00, sven@riska.(none) +12 -12
>     1. Added const keyword at the following places:
>      - input buffer for pretty_print_str
>      - input buffer for write_str
>      - input buffer, end pointer, and return value from sql_ex_info::init
>     2. Fixed the bug by not taking a copy of buf before calling read_str in
>     sql_ex_info::init().
>
>   sql/log_event.h@stripped, 2008-01-30 14:12:34+01:00, sven@riska.(none) +6 -6
>     Added const keyword to fields of the sql_ex_info struct.
>     Added const keyword to arguments and return value of sql_ex_info::init
>
>   sql/sql_binlog.cc@stripped, 2008-01-30 14:12:35+01:00, sven@riska.(none) +3 -4
>     The first BINLOG statement must describe the format for future BINLOG
>     statements. Otherwise, we do not know how to read the BINLOG statement.
>     Problem: only FORMAT_DESCRIPTION_EVENT is currently allowed as the first
>     event. Binlogs from 4.1 begin with a START_EVENT_V3, which serves the
>     same purpose.
>     Fix: We now allow the first BINLOG statement to be a START_EVENT_V3, as
>     well as a FORMAT_DESCRIPTION_EVENT.
>
> diff -Nrup a/mysql-test/suite/binlog/r/binlog_old_versions.result
> b/mysql-test/suite/binlog/r/binlog_old_versions.result
> --- a/mysql-test/suite/binlog/r/binlog_old_versions.result	2008-01-10 16:39:42
> +01:00
> +++ b/mysql-test/suite/binlog/r/binlog_old_versions.result	2008-01-30 14:12:31
> +01:00
> @@ -29,6 +29,16 @@ SELECT COUNT(*) FROM t3;
>  COUNT(*)
>  17920
>  DROP TABLE t1, t2, t3;
> +==== Read binlog from version 4.1 ====
> +SELECT * FROM t1 ORDER BY a;
> +a	b
> +0	last_insert_id
> +4	four
> +190243	random
> +SELECT COUNT(*) FROM t3;
> +COUNT(*)
> +17920
> +DROP TABLE t1, t3;
>  ==== Read binlog from alcatel tree (mysql-5.1-wl2325-5.0-drop6) ====
>  SELECT * FROM t1 ORDER BY a;
>  a	b
> Binary files a/mysql-test/suite/binlog/std_data/binlog_old_version_4_1.000001 and
> b/mysql-test/suite/binlog/std_data/binlog_old_version_4_1.000001 differ
> diff -Nrup a/mysql-test/suite/binlog/t/binlog_old_versions.test
> b/mysql-test/suite/binlog/t/binlog_old_versions.test
> --- a/mysql-test/suite/binlog/t/binlog_old_versions.test	2008-01-10 16:39:42 +01:00
> +++ b/mysql-test/suite/binlog/t/binlog_old_versions.test	2008-01-30 14:12:31 +01:00
> @@ -51,6 +51,21 @@ SELECT COUNT(*) FROM t3;
>  DROP TABLE t1, t2, t3;
>  
>  
> +--echo ==== Read binlog from version 4.1 ====
> +
> +# In this version, neither row-based binlogging nor Xid events
> +# existed, so the binlog was generated without the "row-based tests"
> +# part and the "get xid event" part, and it does not create table t2.
> +
> +# Read binlog.
> +--exec $MYSQL_BINLOG suite/binlog/std_data/binlog_old_version_4_1.000001 | $MYSQL
> +# Show result.
> +SELECT * FROM t1 ORDER BY a;
> +SELECT COUNT(*) FROM t3;
> +# Reset.
> +DROP TABLE t1, t3;
> +
> +
>  --echo ==== Read binlog from alcatel tree (mysql-5.1-wl2325-5.0-drop6) ====
>  
>  # In this version, it was not possible to switch between row-based and
> diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
> --- a/sql/log_event.cc	2008-01-14 16:48:02 +01:00
> +++ b/sql/log_event.cc	2008-01-30 14:12:32 +01:00
> @@ -212,9 +212,9 @@ uint debug_not_change_ts_if_art_event= 1
>  */
>  
>  #ifdef MYSQL_CLIENT
> -static void pretty_print_str(IO_CACHE* cache, char* str, int len)
> +static void pretty_print_str(IO_CACHE* cache, const char* str, int len)
>  {
> -  char* end = str + len;
> +  const char* end = str + len;
>    my_b_printf(cache, "\'");
>    while (str < end)
>    {
> @@ -277,9 +277,9 @@ inline int ignored_error_code(int err_co
>  */
>  
>  #if defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT)
> -static char *pretty_print_str(char *packet, char *str, int len)
> +static char *pretty_print_str(char *packet, const char *str, int len)
>  {
> -  char *end= str + len;
> +  const char *end= str + len;
>    char *pos= packet;
>    *pos++= '\'';
>    while (str < end)
> @@ -388,7 +388,7 @@ static void cleanup_load_tmpdir()
>    write_str()
>  */
>  
> -static bool write_str(IO_CACHE *file, char *str, uint length)
> +static bool write_str(IO_CACHE *file, const char *str, uint length)
>  {
>    uchar tmp[1];
>    tmp[0]= (uchar) length;
> @@ -6011,7 +6011,8 @@ bool sql_ex_info::write_data(IO_CACHE* f
>    sql_ex_info::init()
>  */
>  
> -char *sql_ex_info::init(char *buf, char *buf_end, bool use_new_format)
> +const char *sql_ex_info::init(const char *buf, const char *buf_end,
> +                              bool use_new_format)
>  {
>    cached_new_format = use_new_format;
>    if (use_new_format)
> @@ -6024,12 +6025,11 @@ char *sql_ex_info::init(char *buf, char 
>        the case when we have old format because we will be reusing net buffer
>        to read the actual file before we write out the Create_file event.
>      */
> -    const char *ptr= buf;
> -    if (read_str(&ptr, buf_end, (const char **) &field_term,
> &field_term_len) ||
> -	read_str(&ptr, buf_end, (const char **) &enclosed,   &enclosed_len) ||
> -	read_str(&ptr, buf_end, (const char **) &line_term,  &line_term_len)
> ||
> -	read_str(&ptr, buf_end, (const char **) &line_start, &line_start_len)
> ||
> -	read_str(&ptr, buf_end, (const char **) &escaped,    &escaped_len))
> +    if (read_str(&buf, buf_end, &field_term, &field_term_len) ||
> +        read_str(&buf, buf_end, &enclosed,   &enclosed_len) ||
> +        read_str(&buf, buf_end, &line_term,  &line_term_len) ||
> +        read_str(&buf, buf_end, &line_start, &line_start_len) ||
> +        read_str(&buf, buf_end, &escaped,    &escaped_len))
>        return 0;
>      opt_flags = *buf++;
>    }
> diff -Nrup a/sql/log_event.h b/sql/log_event.h
> --- a/sql/log_event.h	2008-01-10 16:39:42 +01:00
> +++ b/sql/log_event.h	2008-01-30 14:12:34 +01:00
> @@ -152,11 +152,11 @@ struct old_sql_ex
>  struct sql_ex_info
>  {
>    sql_ex_info() {}                            /* Remove gcc warning */
> -  char* field_term;
> -  char* enclosed;
> -  char* line_term;
> -  char* line_start;
> -  char* escaped;
> +  const char* field_term;
> +  const char* enclosed;
> +  const char* line_term;
> +  const char* line_start;
> +  const char* escaped;
>    int cached_new_format;
>    uint8 field_term_len,enclosed_len,line_term_len,line_start_len, escaped_len;
>    char opt_flags;
> @@ -171,7 +171,7 @@ struct sql_ex_info
>  	    line_start_len + escaped_len + 6 : 7);
>    }
>    bool write_data(IO_CACHE* file);
> -  char* init(char* buf,char* buf_end,bool use_new_format);
> +  const char* init(const char* buf, const char* buf_end, bool use_new_format);
>    bool new_format()
>    {
>      return ((cached_new_format != -1) ? cached_new_format :
> diff -Nrup a/sql/sql_binlog.cc b/sql/sql_binlog.cc
> --- a/sql/sql_binlog.cc	2007-12-14 19:01:59 +01:00
> +++ b/sql/sql_binlog.cc	2008-01-30 14:12:35 +01:00
> @@ -159,14 +159,13 @@ void mysql_client_binlog_statement(THD* 
>        */
>        if (!have_fd_event)
>        {
> -        if (bufptr[EVENT_TYPE_OFFSET] == FORMAT_DESCRIPTION_EVENT)
> +        int type = bufptr[EVENT_TYPE_OFFSET];
> +        if (type == FORMAT_DESCRIPTION_EVENT || type == START_EVENT_V3)

Just an info note. After WL#4241 will be done the slave will be aware
of the master's version at any point not just at connection time as
currently.
This would apply to your case to allow some assert like

  assert (type != START_EVENT_V3 || master_version < 4.1)

>            have_fd_event= TRUE;
>          else
>          {
>            my_error(ER_NO_FORMAT_DESCRIPTION_EVENT_BEFORE_BINLOG_STATEMENT,
> -                   MYF(0),
> -                   Log_event::get_type_str(
> -                     (Log_event_type)bufptr[EVENT_TYPE_OFFSET]));
> +                   MYF(0), Log_event::get_type_str((Log_event_type)type));
>            goto end;
>          }
>        }
>

Thread
bk commit into 5.1 tree (sven:1.2678) BUG#34141Sven Sandberg30 Jan
  • Re: bk commit into 5.1 tree (sven:1.2678) BUG#34141Andrei Elkin30 Jan