Re: bk commit into 5.1 tree (sven:1.2585) BUG#31581

[Mats Kindahl]

Hi Sven!

Review of the first batch below.

Just my few cents,
Mats Kindahl

Sven Sandberg wrote:
> 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-10-31 11:09:17+01:00, sven@murkla.(none) +2 -0
>   BUG#31581: 5.1-telco-6.1 -> 5.1.22. Slave crashes during starting
>   made it compile and work exactly like before the first patch.
>
>   sql/log_event_old.cc@stripped, 2007-10-31 11:09:13+01:00, sven@murkla.(none) +104 -217
>           - merge some constructors into one
>     
>           - rename get_type_code to xget_type_code and TYPE_CODE to
>             XTYPE_CODE in copied code and declare a pure virtual
>             xget_type_code in Log_event_old
>   

Why?

>     
>           - remove static functions last_uniq_key() and record_compare(),
>             which were defined in both log_event.cc and log_event_old.cc
>             and copied another time to log_event_old.cc in previous patch, as
>             well as the non-static duplicate_key_error() which was copied
>             from log_event.cc to log_event_old.cc but not used in
>             log_event_old.cc.
>   

Why?

>   sql/log_event_old.h@stripped, 2007-10-31 11:09:13+01:00, sven@murkla.(none) +36 -97
>           - merge some constructors into one; remove some destructors
>     
>           - rename get_type_code to xget_type_code and TYPE_CODE to
>             XTYPE_CODE in copied code and declare a pure virtual
>             xget_type_code in Log_event_old
>   

Why?

>     
>           - remove `friend class Old_rows_log_event' from Rows_log_event
>             and Old_rows_log_event
>   

Thanks! Gratuitous friend classes cause a certain problem with 
dependencies between classes.

>     
>           - make Old_rows_log_event inherit from Log_event and
>             *_rows_log_event_old inherit from Old_rows_log_event.
>   

OK.

> diff -Nrup a/sql/log_event_old.cc b/sql/log_event_old.cc
> --- a/sql/log_event_old.cc	2007-10-29 14:07:36 +01:00
> +++ b/sql/log_event_old.cc	2007-10-31 11:09:13 +01:00
> @@ -11,9 +11,9 @@
>  
>  // Old implementation of do_apply_event()
>  int 
> -Old_rows_log_event::do_apply_event(Rows_log_event *ev, const Relay_log_info *rli)
> +Old_rows_log_event::do_apply_event(Old_rows_log_event *ev, const Relay_log_info
> *rli)
>  {
>   

Why do this one have the event as the first argument? If this is a 
static method, then it is clearly not correct since the event instance 
has to be passed as first parameter.

> -  DBUG_ENTER("Rows_log_event::do_apply_event(st_relay_log_info*)");
> +  DBUG_ENTER("Old_rows_log_event::do_apply_event(st_relay_log_info*)");
>    int error= 0;
>    THD *thd= ev->thd;
>    uchar const *row_start= ev->m_rows_buf;
> @@ -30,7 +30,7 @@ Old_rows_log_event::do_apply_event(Rows_
>         This one is supposed to be set: just an extra check so that
>         nothing strange has happened.
>       */
> -    DBUG_ASSERT(ev->get_flags(Rows_log_event::STMT_END_F));
> +    DBUG_ASSERT(ev->get_flags(Old_rows_log_event::STMT_END_F));
>  
>      const_cast<Relay_log_info*>(rli)->clear_tables_to_lock();
>      close_thread_tables(thd);
> @@ -148,7 +148,7 @@ Old_rows_log_event::do_apply_event(Rows_
>            thd->lock= 0;
>            thd->query_error= 1;
>            const_cast<Relay_log_info*>(rli)->clear_tables_to_lock();
> -          DBUG_RETURN(Rows_log_event::ERR_BAD_TABLE_DEF);
> +          DBUG_RETURN(Old_rows_log_event::ERR_BAD_TABLE_DEF);
>          }
>        }
>      }
> @@ -163,8 +163,8 @@ Old_rows_log_event::do_apply_event(Rows_
>        TODO [/Matz]: Maybe the query cache should not be invalidated
>        here? It might be that a table is not changed, even though it
>        was locked for the statement.  We do know that each
> -      Rows_log_event contain at least one row, so after processing one
> -      Rows_log_event, we can invalidate the query cache for the
> +      Old_rows_log_event contain at least one row, so after processing one
> +      Old_rows_log_event, we can invalidate the query cache for the
>        associated table.
>       */
>      for (TABLE_LIST *ptr= rli->tables_to_lock ; ptr ; ptr= ptr->next_global)
> @@ -200,12 +200,12 @@ Old_rows_log_event::do_apply_event(Rows_
>        Make sure to set/clear them before executing the main body of
>        the event.
>      */
> -    if (ev->get_flags(Rows_log_event::NO_FOREIGN_KEY_CHECKS_F))
> +    if (ev->get_flags(Old_rows_log_event::NO_FOREIGN_KEY_CHECKS_F))
>          thd->options|= OPTION_NO_FOREIGN_KEY_CHECKS;
>      else
>          thd->options&= ~OPTION_NO_FOREIGN_KEY_CHECKS;
>  
> -    if (ev->get_flags(Rows_log_event::RELAXED_UNIQUE_CHECKS_F))
> +    if (ev->get_flags(Old_rows_log_event::RELAXED_UNIQUE_CHECKS_F))
>          thd->options|= OPTION_RELAXED_UNIQUE_CHECKS;
>      else
>          thd->options&= ~OPTION_RELAXED_UNIQUE_CHECKS;
> @@ -275,7 +275,7 @@ Old_rows_log_event::do_apply_event(Rows_
>      We need to delay this clear until the table def is no longer needed.
>      The table def is needed in unpack_row().
>    */
> -  if (rli->tables_to_lock &&
> ev->get_flags(Rows_log_event::STMT_END_F))
> +  if (rli->tables_to_lock &&
> ev->get_flags(Old_rows_log_event::STMT_END_F))
>      const_cast<Relay_log_info*>(rli)->clear_tables_to_lock();
>  
>    if (error)
> @@ -311,7 +311,7 @@ Old_rows_log_event::do_apply_event(Rows_
>    */
>    if (table && (table->s->primary_key == MAX_KEY) &&
>        !ev->cache_stmt && 
> -      ev->get_flags(Rows_log_event::STMT_END_F) == Rows_log_event::RLE_NO_FLAGS)
> +      ev->get_flags(Old_rows_log_event::STMT_END_F) ==
> Old_rows_log_event::RLE_NO_FLAGS)
>    {
>      /*
>        ------------ Temporary fix until WL#2975 is implemented ---------
> @@ -323,7 +323,7 @@ Old_rows_log_event::do_apply_event(Rows_
>        present, and idempotency is not guaranteed (no PK) so we risk
>        that repeating leads to double insert. So we desperately try to
>        continue, hope we'll eventually leave this buggy situation (by
> -      executing the final Rows_log_event). If we are in a hopeless
> +      executing the final Old_rows_log_event). If we are in a hopeless
>        wait (reached end of last relay log and nothing gets appended
>        there), we timeout after one minute, and notify DBA about the
>        problem.  When WL#2975 is implemented, just remove the member
> @@ -1224,8 +1224,9 @@ int Update_rows_log_event_old::do_exec_r
>  **************************************************************************/
>  
>  #ifndef MYSQL_CLIENT
> -Rows_log_event::Rows_log_event(THD *thd_arg, TABLE *tbl_arg, ulong tid,
> -                               MY_BITMAP const *cols, bool is_transactional)
> +Old_rows_log_event::Old_rows_log_event(THD *thd_arg, TABLE *tbl_arg, ulong tid,
> +                                       MY_BITMAP const *cols,
> +                                       bool is_transactional)
>    : Log_event(thd_arg, 0, is_transactional),
>      m_row_count(0),
>      m_table(tbl_arg),
> @@ -1270,10 +1271,10 @@ Rows_log_event::Rows_log_event(THD *thd_
>  }
>  #endif
>  
> -Rows_log_event::Rows_log_event(const char *buf, uint event_len,
> -                               Log_event_type event_type,
> -                               const Format_description_log_event
> -                               *description_event)
> +Old_rows_log_event::Old_rows_log_event(const char *buf, uint event_len,
> +                                       Log_event_type event_type,
> +                                       const Format_description_log_event
> +                                       *description_event)
>    : Log_event(buf, description_event),
>      m_row_count(0),
>  #ifndef MYSQL_CLIENT
> @@ -1284,7 +1285,7 @@ Rows_log_event::Rows_log_event(const cha
>      ,m_key(NULL), m_curr_row(NULL), m_curr_row_end(NULL)
>  #endif
>  {
> -  DBUG_ENTER("Rows_log_event::Rows_log_event(const char*,...)");
> +  DBUG_ENTER("Old_rows_log_event::Old_Rows_log_event(const char*,...)");
>    uint8 const common_header_len= description_event->common_header_len;
>    uint8 const post_header_len= description_event->post_header_len[event_type-1];
>  
> @@ -1384,7 +1385,7 @@ Rows_log_event::Rows_log_event(const cha
>    DBUG_VOID_RETURN;
>  }
>  
> -Rows_log_event::~Rows_log_event()
> +Old_rows_log_event::~Old_rows_log_event()
>  {
>    if (m_cols.bitmap == m_bitbuf) // no my_malloc happened
>      m_cols.bitmap= 0; // so no my_free in bitmap_free
> @@ -1392,9 +1393,9 @@ Rows_log_event::~Rows_log_event()
>    my_free((uchar*)m_rows_buf, MYF(MY_ALLOW_ZERO_PTR));
>  }
>  
> -int Rows_log_event::get_data_size()
> +int Old_rows_log_event::get_data_size()
>  {
> -  int const type_code= get_type_code();
> +  int const type_code= xget_type_code();
>  
>    uchar buf[sizeof(m_width)+1];
>    uchar *end= net_store_length(buf, (m_width + 7) / 8);
> @@ -1416,14 +1417,14 @@ int Rows_log_event::get_data_size()
>  
>  
>  #ifndef MYSQL_CLIENT
> -int Rows_log_event::do_add_row_data(uchar *row_data, size_t length)
> +int Old_rows_log_event::do_add_row_data(uchar *row_data, size_t length)
>  {
>    /*
>      When the table has a primary key, we would probably want, by default, to
>      log only the primary key value instead of the entire "before image". This
>      would save binlog space. TODO
>    */
> -  DBUG_ENTER("Rows_log_event::do_add_row_data");
> +  DBUG_ENTER("Old_rows_log_event::do_add_row_data");
>    DBUG_PRINT("enter", ("row_data: 0x%lx  length: %lu", (ulong) row_data,
>                         (ulong) length));
>    /*
> @@ -1474,9 +1475,9 @@ int Rows_log_event::do_add_row_data(ucha
>  #endif
>  
>  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
> -int Rows_log_event::do_apply_event(Relay_log_info const *rli)
> +int Old_rows_log_event::do_apply_event(Relay_log_info const *rli)
>  {
> -  DBUG_ENTER("Rows_log_event::do_apply_event(Relay_log_info*)");
> +  DBUG_ENTER("Old_rows_log_event::do_apply_event(Relay_log_info*)");
>    int error= 0;
>  
>    /*
> @@ -1624,8 +1625,8 @@ int Rows_log_event::do_apply_event(Relay
>        TODO [/Matz]: Maybe the query cache should not be invalidated
>        here? It might be that a table is not changed, even though it
>        was locked for the statement.  We do know that each
> -      Rows_log_event contain at least one row, so after processing one
> -      Rows_log_event, we can invalidate the query cache for the
> +      Old_rows_log_event contain at least one row, so after processing one
> +      Old_rows_log_event, we can invalidate the query cache for the
>        associated table.
>       */
>      for (TABLE_LIST *ptr= rli->tables_to_lock ; ptr ; ptr= ptr->next_global)
> @@ -1827,7 +1828,7 @@ int Rows_log_event::do_apply_event(Relay
>        present, and idempotency is not guaranteed (no PK) so we risk
>        that repeating leads to double insert. So we desperately try to
>        continue, hope we'll eventually leave this buggy situation (by
> -      executing the final Rows_log_event). If we are in a hopeless
> +      executing the final Old_rows_log_event). If we are in a hopeless
>        wait (reached end of last relay log and nothing gets appended
>        there), we timeout after one minute, and notify DBA about the
>        problem.  When WL#2975 is implemented, just remove the member
> @@ -1840,7 +1841,7 @@ int Rows_log_event::do_apply_event(Relay
>  }
>  
>  Log_event::enum_skip_reason
> -Rows_log_event::do_shall_skip(Relay_log_info *rli)
> +Old_rows_log_event::do_shall_skip(Relay_log_info *rli)
>  {
>    /*
>      If the slave skip counter is 1 and this event does not end a
> @@ -1854,9 +1855,9 @@ Rows_log_event::do_shall_skip(Relay_log_
>  }
>  
>  int
> -Rows_log_event::do_update_pos(Relay_log_info *rli)
> +Old_rows_log_event::do_update_pos(Relay_log_info *rli)
>  {
> -  DBUG_ENTER("Rows_log_event::do_update_pos");
> +  DBUG_ENTER("Old_rows_log_event::do_update_pos");
>    int error= 0;
>  
>    DBUG_PRINT("info", ("flags: %s",
> @@ -1916,7 +1917,7 @@ Rows_log_event::do_update_pos(Relay_log_
>        /*
>          Clear any errors pushed in thd->net.last_err* if for example "no key
>          found" (as this is allowed). This is a safety measure; apparently
> -        those errors (e.g. when executing a Delete_rows_log_event of a
> +        those errors (e.g. when executing a Delete_rows_log_event_old of a
>          non-existing row, like in rpl_row_mystery22.test,
>          thd->net.last_error = "Can't find record in 't1'" and last_errno=1032)
>          do not become visible. We still prefer to wipe them out.
> @@ -1941,7 +1942,7 @@ Rows_log_event::do_update_pos(Relay_log_
>  #endif /* !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) */
>  
>  #ifndef MYSQL_CLIENT
> -bool Rows_log_event::write_data_header(IO_CACHE *file)
> +bool Old_rows_log_event::write_data_header(IO_CACHE *file)
>  {
>    uchar buf[ROWS_HEADER_LEN];	// No need to init the buffer
>    DBUG_ASSERT(m_table_id != ~0UL);
> @@ -1956,7 +1957,7 @@ bool Rows_log_event::write_data_header(I
>    return (my_b_safe_write(file, buf, ROWS_HEADER_LEN));
>  }
>  
> -bool Rows_log_event::write_data_body(IO_CACHE*file)
> +bool Old_rows_log_event::write_data_body(IO_CACHE*file)
>  {
>    /*
>       Note that this should be the number of *bits*, not the number of
> @@ -1977,7 +1978,7 @@ bool Rows_log_event::write_data_body(IO_
>    /*
>      TODO[refactor write]: Remove the "down cast" here (and elsewhere).
>     */
> -  if (get_type_code() == UPDATE_ROWS_EVENT)
> +  if (xget_type_code() == UPDATE_ROWS_EVENT)
>   

Why? There are dedicated event type codes associated with this, so use 
them instead.

>    {
>      DBUG_DUMP("m_cols_ai", (uchar*) m_cols_ai.bitmap,
>                no_bytes_in_map(&m_cols_ai));
> @@ -1993,7 +1994,7 @@ bool Rows_log_event::write_data_body(IO_
>  #endif
>  
>  #if defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT)
> -void Rows_log_event::pack_info(Protocol *protocol)
> +void Old_rows_log_event::pack_info(Protocol *protocol)
>  {
>    char buf[256];
>    char const *const flagstr=
> @@ -2005,9 +2006,9 @@ void Rows_log_event::pack_info(Protocol 
>  #endif
>  
>  #ifdef MYSQL_CLIENT
> -void Rows_log_event::print_helper(FILE *file,
> -                                  PRINT_EVENT_INFO *print_event_info,
> -                                  char const *const name)
> +void Old_rows_log_event::print_helper(FILE *file,
> +                                      PRINT_EVENT_INFO *print_event_info,
> +                                      char const *const name)
>  {
>    IO_CACHE *const head= &print_event_info->head_cache;
>    IO_CACHE *const body= &print_event_info->body_cache;
> @@ -2037,11 +2038,12 @@ void Rows_log_event::print_helper(FILE *
>    Constructor used to build an event for writing to the binary log.
>   */
>  #if !defined(MYSQL_CLIENT)
> -Write_rows_log_event::Write_rows_log_event(THD *thd_arg, TABLE *tbl_arg,
> -                                           ulong tid_arg,
> -                                           MY_BITMAP const *cols,
> -                                           bool is_transactional)
> -  : Rows_log_event(thd_arg, tbl_arg, tid_arg, cols, is_transactional)
> +Write_rows_log_event_old::Write_rows_log_event_old(THD *thd_arg,
> +                                                   TABLE *tbl_arg,
> +                                                   ulong tid_arg,
> +                                                   MY_BITMAP const *cols,
> +                                                   bool is_transactional)
> +  : Old_rows_log_event(thd_arg, tbl_arg, tid_arg, cols, is_transactional)
>  {
>  }
>  #endif
> @@ -2050,17 +2052,18 @@ Write_rows_log_event::Write_rows_log_eve
>    Constructor used by slave to read the event from the binary log.
>   */
>  #ifdef HAVE_REPLICATION
> -Write_rows_log_event::Write_rows_log_event(const char *buf, uint event_len,
> -                                           const Format_description_log_event
> -                                           *description_event)
> -: Rows_log_event(buf, event_len, WRITE_ROWS_EVENT, description_event)
> +Write_rows_log_event_old::Write_rows_log_event_old(const char *buf,
> +                                                   uint event_len,
> +                                                   const
> Format_description_log_event
> +                                                   *description_event)
> +: Old_rows_log_event(buf, event_len, WRITE_ROWS_EVENT, description_event)
>   

No, these are PRE_GA_WRITE_ROWS_EVENT.

>  {
>  }
>  #endif
>  
>  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
>  int 
> -Write_rows_log_event::do_before_row_operations(const Slave_reporting_capability
> *const)
> +Write_rows_log_event_old::do_before_row_operations(const Slave_reporting_capability
> *const)
>  {
>    int error= 0;
>  
> @@ -2122,8 +2125,8 @@ Write_rows_log_event::do_before_row_oper
>  }
>  
>  int 
> -Write_rows_log_event::do_after_row_operations(const Slave_reporting_capability
> *const, 
> -                                              int error)
> +Write_rows_log_event_old::do_after_row_operations(const Slave_reporting_capability
> *const,
> +                                                  int error)
>  {
>    int local_error= 0;
>    m_table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
> @@ -2142,44 +2145,6 @@ Write_rows_log_event::do_after_row_opera
>  }
>  
>  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
> -
> -/*
> -  Check if there are more UNIQUE keys after the given key.
> -*/
> -static int
> -last_uniq_key(TABLE *table, uint keyno)
> -{
> -  while (++keyno < table->s->keys)
> -    if (table->key_info[keyno].flags & HA_NOSAME)
> -      return 0;
> -  return 1;
> -}
> -
> -/**
> -   Check if an error is a duplicate key error.
> -
> -   This function is used to check if an error code is one of the
> -   duplicate key error, i.e., and error code for which it is sensible
> -   to do a <code>get_dup_key()</code> to retrieve the duplicate key.
> -
> -   @param errcode The error code to check.
> -
> -   @return <code>true</code> if the error code is such that
> -   <code>get_dup_key()</code> will return true,
> <code>false</code>
> -   otherwise.
> - */
> -bool
> -is_duplicate_key_error(int errcode)
> -{
> -  switch (errcode)
> -  {
> -  case HA_ERR_FOUND_DUPP_KEY:
> -  case HA_ERR_FOUND_DUPP_UNIQUE:
> -    return true;
> -  }
> -  return false;
> -}
> -
>  /**
>    Write the current row into event's table.
>  
> @@ -2216,8 +2181,8 @@ is_duplicate_key_error(int errcode)
>  */ 
>  
>  int
> -Rows_log_event::write_row(const Relay_log_info *const rli,
> -                          const bool overwrite)
> +Old_rows_log_event::write_row(const Relay_log_info *const rli,
> +                              const bool overwrite)
>  {
>    DBUG_ENTER("write_row");
>    DBUG_ASSERT(m_table != NULL && thd != NULL);
> @@ -2403,7 +2368,7 @@ Rows_log_event::write_row(const Relay_lo
>  #endif
>  
>  int 
> -Write_rows_log_event::do_exec_row(const Relay_log_info *const rli)
> +Write_rows_log_event_old::do_exec_row(const Relay_log_info *const rli)
>  {
>    DBUG_ASSERT(m_table != NULL);
>    int error= write_row(rli, TRUE /* overwrite */);
> @@ -2417,9 +2382,10 @@ Write_rows_log_event::do_exec_row(const 
>  #endif /* !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) */
>  
>  #ifdef MYSQL_CLIENT
> -void Write_rows_log_event::print(FILE *file, PRINT_EVENT_INFO* print_event_info)
> +void Write_rows_log_event_old::print(FILE *file,
> +                                     PRINT_EVENT_INFO* print_event_info)
>  {
> -  Rows_log_event::print_helper(file, print_event_info, "Write_rows");
> +  Old_rows_log_event::print_helper(file, print_event_info, "Write_rows_old");
>  }
>  #endif
>  
> @@ -2428,84 +2394,6 @@ void Write_rows_log_event::print(FILE *f
>  **************************************************************************/
>  
>  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
> -/*
> -  Compares table->record[0] and table->record[1]
> -
> -  Returns TRUE if different.
> -*/
> -static bool record_compare(TABLE *table)
> -{
> -  /*
> -    Need to set the X bit and the filler bits in both records since
> -    there are engines that do not set it correctly.
> -
> -    In addition, since MyISAM checks that one hasn't tampered with the
> -    record, it is necessary to restore the old bytes into the record
> -    after doing the comparison.
> -
> -    TODO[record format ndb]: Remove it once NDB returns correct
> -    records. Check that the other engines also return correct records.
> -   */
> -
> -  bool result= FALSE;
> -  uchar saved_x[2], saved_filler[2];
> -
> -  if (table->s->null_bytes > 0)
> -  {
> -    for (int i = 0 ; i < 2 ; ++i)
> -    {
> -      saved_x[i]= table->record[i][0];
> -      saved_filler[i]= table->record[i][table->s->null_bytes - 1];
> -      table->record[i][0]|= 1U;
> -      table->record[i][table->s->null_bytes - 1]|=
> -        256U - (1U << table->s->last_null_bit_pos);
> -    }
> -  }
> -
> -  if (table->s->blob_fields + table->s->varchar_fields == 0)
> -  {
> -    result= cmp_record(table,record[1]);
> -    goto record_compare_exit;
> -  }
> -
> -  /* Compare null bits */
> -  if (memcmp(table->null_flags,
> -	     table->null_flags+table->s->rec_buff_length,
> -	     table->s->null_bytes))
> -  {
> -    result= TRUE;				// Diff in NULL value
> -    goto record_compare_exit;
> -  }
> -
> -  /* Compare updated fields */
> -  for (Field **ptr=table->field ; *ptr ; ptr++)
> -  {
> -    if ((*ptr)->cmp_binary_offset(table->s->rec_buff_length))
> -    {
> -      result= TRUE;
> -      goto record_compare_exit;
> -    }
> -  }
> -
> -record_compare_exit:
> -  /*
> -    Restore the saved bytes.
> -
> -    TODO[record format ndb]: Remove this code once NDB returns the
> -    correct record format.
> -  */
> -  if (table->s->null_bytes > 0)
> -  {
> -    for (int i = 0 ; i < 2 ; ++i)
> -    {
> -      table->record[i][0]= saved_x[i];
> -      table->record[i][table->s->null_bytes - 1]= saved_filler[i];
> -    }
> -  }
> -
> -  return result;
> -}
> -
>  /**
>    Locate the current row in event's table.
>  
> @@ -2533,7 +2421,7 @@ record_compare_exit:
>    @c position() and @c rnd_pos() will be used. 
>   */
>  
> -int Rows_log_event::find_row(const Relay_log_info *rli)
> +int Old_rows_log_event::find_row(const Relay_log_info *rli)
>  {
>    DBUG_ENTER("find_row");
>  
> @@ -2778,10 +2666,13 @@ int Rows_log_event::find_row(const Relay
>   */
>  
>  #ifndef MYSQL_CLIENT
> -Delete_rows_log_event::Delete_rows_log_event(THD *thd_arg, TABLE *tbl_arg,
> -                                             ulong tid, MY_BITMAP const *cols,
> -                                             bool is_transactional)
> -  : Rows_log_event(thd_arg, tbl_arg, tid, cols, is_transactional)
> +Delete_rows_log_event_old::Delete_rows_log_event_old(THD *thd_arg,
> +                                                     TABLE *tbl_arg,
> +                                                     ulong tid,
> +                                                     MY_BITMAP const *cols,
> +                                                     bool is_transactional)
> +  : Old_rows_log_event(thd_arg, tbl_arg, tid, cols, is_transactional),
> +    m_after_image(NULL), m_memory(NULL)
>  {
>  }
>  #endif /* #if !defined(MYSQL_CLIENT) */
> @@ -2790,10 +2681,12 @@ Delete_rows_log_event::Delete_rows_log_e
>    Constructor used by slave to read the event from the binary log.
>   */
>  #ifdef HAVE_REPLICATION
> -Delete_rows_log_event::Delete_rows_log_event(const char *buf, uint event_len,
> -                                             const Format_description_log_event
> -                                             *description_event)
> -  : Rows_log_event(buf, event_len, DELETE_ROWS_EVENT, description_event)
> +Delete_rows_log_event_old::Delete_rows_log_event_old(const char *buf,
> +                                                     uint event_len,
> +                                                     const
> Format_description_log_event
> +                                                     *description_event)
> +  : Old_rows_log_event(buf, event_len, DELETE_ROWS_EVENT, description_event),
>   

No, these are PRE_GA_DELETE_ROWS_EVENT.

> +    m_after_image(NULL), m_memory(NULL)
>  {
>  }
>  #endif
> @@ -2801,7 +2694,7 @@ Delete_rows_log_event::Delete_rows_log_e
>  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
>  
>  int 
> -Delete_rows_log_event::do_before_row_operations(const Slave_reporting_capability
> *const)
> +Delete_rows_log_event_old::do_before_row_operations(const Slave_reporting_capability
> *const)
>  {
>    if ((m_table->file->ha_table_flags() &
> HA_PRIMARY_KEY_REQUIRED_FOR_POSITION) &&
>        m_table->s->primary_key < MAX_KEY)
> @@ -2823,8 +2716,8 @@ Delete_rows_log_event::do_before_row_ope
>  }
>  
>  int 
> -Delete_rows_log_event::do_after_row_operations(const Slave_reporting_capability
> *const, 
> -                                               int error)
> +Delete_rows_log_event_old::do_after_row_operations(const Slave_reporting_capability
> *const,
> +                                                   int error)
>  {
>    /*error= ToDo:find out what this should really be, this triggers close_scan in
> nbd, returning error?*/
>    m_table->file->ha_index_or_rnd_end();
> @@ -2834,7 +2727,7 @@ Delete_rows_log_event::do_after_row_oper
>    return error;
>  }
>  
> -int Delete_rows_log_event::do_exec_row(const Relay_log_info *const rli)
> +int Delete_rows_log_event_old::do_exec_row(const Relay_log_info *const rli)
>  {
>    int error;
>    DBUG_ASSERT(m_table != NULL);
> @@ -2852,10 +2745,10 @@ int Delete_rows_log_event::do_exec_row(c
>  #endif /* !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) */
>  
>  #ifdef MYSQL_CLIENT
> -void Delete_rows_log_event::print(FILE *file,
> -                                  PRINT_EVENT_INFO* print_event_info)
> +void Delete_rows_log_event_old::print(FILE *file,
> +                                      PRINT_EVENT_INFO* print_event_info)
>  {
> -  Rows_log_event::print_helper(file, print_event_info, "Delete_rows");
> +  Old_rows_log_event::print_helper(file, print_event_info, "Delete_rows_old");
>  }
>  #endif
>  
> @@ -2868,26 +2761,18 @@ void Delete_rows_log_event::print(FILE *
>    Constructor used to build an event for writing to the binary log.
>   */
>  #if !defined(MYSQL_CLIENT)
> -Update_rows_log_event::Update_rows_log_event(THD *thd_arg, TABLE *tbl_arg,
> -                                             ulong tid,
> -                                             MY_BITMAP const *cols_bi,
> -                                             MY_BITMAP const *cols_ai,
> -                                             bool is_transactional)
> -: Rows_log_event(thd_arg, tbl_arg, tid, cols_bi, is_transactional)
> -{
> -  init(cols_ai);
> -}
> -
> -Update_rows_log_event::Update_rows_log_event(THD *thd_arg, TABLE *tbl_arg,
> -                                             ulong tid,
> -                                             MY_BITMAP const *cols,
> -                                             bool is_transactional)
> -: Rows_log_event(thd_arg, tbl_arg, tid, cols, is_transactional)
> +Update_rows_log_event_old::Update_rows_log_event_old(THD *thd_arg,
> +                                                     TABLE *tbl_arg,
> +                                                     ulong tid,
> +                                                     MY_BITMAP const *cols,
> +                                                     bool is_transactional)
> +  : Old_rows_log_event(thd_arg, tbl_arg, tid, cols, is_transactional),
> +    m_after_image(NULL), m_memory(NULL)
>  {
>    init(cols);
>  }
>  
> -void Update_rows_log_event::init(MY_BITMAP const *cols)
> +void Update_rows_log_event_old::init(MY_BITMAP const *cols)
>  {
>    /* if bitmap_init fails, caught in is_valid() */
>    if (likely(!bitmap_init(&m_cols_ai,
> @@ -2906,7 +2791,7 @@ void Update_rows_log_event::init(MY_BITM
>  #endif /* !defined(MYSQL_CLIENT) */
>  
>  
> -Update_rows_log_event::~Update_rows_log_event()
> +Update_rows_log_event_old::~Update_rows_log_event_old()
>  {
>    if (m_cols_ai.bitmap == m_bitbuf_ai) // no my_malloc happened
>      m_cols_ai.bitmap= 0; // so no my_free in bitmap_free
> @@ -2918,11 +2803,13 @@ Update_rows_log_event::~Update_rows_log_
>    Constructor used by slave to read the event from the binary log.
>   */
>  #ifdef HAVE_REPLICATION
> -Update_rows_log_event::Update_rows_log_event(const char *buf, uint event_len,
> -                                             const
> -                                             Format_description_log_event
> -                                             *description_event)
> -  : Rows_log_event(buf, event_len, UPDATE_ROWS_EVENT, description_event)
> +Update_rows_log_event_old::Update_rows_log_event_old(const char *buf,
> +                                                     uint event_len,
> +                                                     const
> +                                                     Format_description_log_event
> +                                                     *description_event)
> +  : Old_rows_log_event(buf, event_len, UPDATE_ROWS_EVENT, description_event),
>   

PRE_GA_UPDATE_ROWS_EVENT.

> +    m_after_image(NULL), m_memory(NULL)
>  {
>  }
>  #endif
> @@ -2930,7 +2817,7 @@ Update_rows_log_event::Update_rows_log_e
>  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
>  
>  int 
> -Update_rows_log_event::do_before_row_operations(const Slave_reporting_capability
> *const)
> +Update_rows_log_event_old::do_before_row_operations(const Slave_reporting_capability
> *const)
>  {
>    if (m_table->s->keys > 0)
>    {
> @@ -2946,8 +2833,8 @@ Update_rows_log_event::do_before_row_ope
>  }
>  
>  int 
> -Update_rows_log_event::do_after_row_operations(const Slave_reporting_capability
> *const, 
> -                                               int error)
> +Update_rows_log_event_old::do_after_row_operations(const Slave_reporting_capability
> *const,
> +                                                   int error)
>  {
>    /*error= ToDo:find out what this should really be, this triggers close_scan in
> nbd, returning error?*/
>    m_table->file->ha_index_or_rnd_end();
> @@ -2958,7 +2845,7 @@ Update_rows_log_event::do_after_row_oper
>  }
>  
>  int 
> -Update_rows_log_event::do_exec_row(const Relay_log_info *const rli)
> +Update_rows_log_event_old::do_exec_row(const Relay_log_info *const rli)
>  {
>    DBUG_ASSERT(m_table != NULL);
>  
> @@ -3014,9 +2901,9 @@ Update_rows_log_event::do_exec_row(const
>  #endif /* !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) */
>  
>  #ifdef MYSQL_CLIENT
> -void Update_rows_log_event::print(FILE *file,
> -				  PRINT_EVENT_INFO* print_event_info)
> +void Update_rows_log_event_old::print(FILE *file,
> +                                      PRINT_EVENT_INFO* print_event_info)
>  {
> -  Rows_log_event::print_helper(file, print_event_info, "Update_rows");
> +  Old_rows_log_event::print_helper(file, print_event_info, "Update_rows_old");
>  }
>  #endif
> diff -Nrup a/sql/log_event_old.h b/sql/log_event_old.h
> --- a/sql/log_event_old.h	2007-10-29 14:07:36 +01:00
> +++ b/sql/log_event_old.h	2007-10-31 11:09:13 +01:00
> @@ -21,10 +21,11 @@
>   */
>  
>    
> -class Old_rows_log_event
> +class Old_rows_log_event : public Log_event
>  {
>    /********** BEGIN CUT & PASTE FROM Rows_log_event **********/
>  public:
> +  virtual Log_event_type xget_type_code() = 0;
>   

Why? Why not use the original get_type_code() and override that one?

>    /**
>       Enumeration of the errors that can be returned.
>     */
> @@ -71,7 +72,7 @@ public:
>        RLE_NO_FLAGS = 0U
>    };
>  
> -  virtual ~Rows_log_event();
> +  virtual ~Old_rows_log_event();
>  
>    void set_flags(flag_set flags_arg) { m_flags |= flags_arg; }
>    void clear_flags(flag_set flags_arg) { m_flags &= ~flags_arg; }
> @@ -107,8 +108,8 @@ public:
>  #endif
>    /*
>      Check that malloc() succeeded in allocating memory for the rows
> -    buffer and the COLS vector. Checking that an Update_rows_log_event
> -    is valid is done in the Update_rows_log_event::is_valid()
> +    buffer and the COLS vector. Checking that an Update_rows_log_event_old
> +    is valid is done in the Update_rows_log_event_old::is_valid()
>      function.
>    */
>    virtual bool is_valid() const
> @@ -124,12 +125,12 @@ protected:
>       this class, not create instances of this class.
>    */
>  #ifndef MYSQL_CLIENT
> -  Rows_log_event(THD*, TABLE*, ulong table_id, 
> -		 MY_BITMAP const *cols, bool is_transactional);
> +  Old_rows_log_event(THD*, TABLE*, ulong table_id,
> +                     MY_BITMAP const *cols, bool is_transactional);
>  #endif
> -  Rows_log_event(const char *row_data, uint event_len, 
> -		 Log_event_type event_type,
> -		 const Format_description_log_event *description_event);
> +  Old_rows_log_event(const char *row_data, uint event_len,
> +                     Log_event_type event_type,
> +                     const Format_description_log_event *description_event);
>  
>  #ifdef MYSQL_CLIENT
>    void print_helper(FILE *, PRINT_EVENT_INFO *, char const *const name);
> @@ -247,17 +248,12 @@ private:
>    virtual int do_exec_row(const Relay_log_info *const rli) = 0;
>  #endif /* !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) */
>  
> -  friend class Old_rows_log_event;
>    /********** END OF CUT & PASTE FROM Rows_log_event **********/
> - public:
> - 
> -  virtual ~Old_rows_log_event() {}
> -
>   protected:
>    
>  #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
>  
> -  int do_apply_event(Rows_log_event*,const Relay_log_info*);
> +  int do_apply_event(Old_rows_log_event*,const Relay_log_info*);
>  
>    /*
>      Primitive to prepare for a sequence of row executions.
> @@ -326,24 +322,23 @@ private:
>  };
>  
>  
> -class Write_rows_log_event_old 
> - : public Write_rows_log_event, public Old_rows_log_event
> +class Write_rows_log_event_old : public Old_rows_log_event
>  {
>    /********** BEGIN CUT & PASTE FROM Write_rows_log_event **********/
>  public:
>    enum 
>    {
>      /* Support interface to THD::binlog_prepare_pending_rows_event */
> -    TYPE_CODE = WRITE_ROWS_EVENT
> +    XTYPE_CODE = WRITE_ROWS_EVENT
>   

Why? Also, it should be PRE_GA_WRITE_ROWS_EVENT.

>    };
>  
>  #if !defined(MYSQL_CLIENT)
> -  Write_rows_log_event(THD*, TABLE*, ulong table_id, 
> -		       MY_BITMAP const *cols, bool is_transactional);
> +  Write_rows_log_event_old(THD*, TABLE*, ulong table_id,
> +                           MY_BITMAP const *cols, bool is_transactional);
>  #endif
>  #ifdef HAVE_REPLICATION
> -  Write_rows_log_event(const char *buf, uint event_len, 
> -                       const Format_description_log_event *description_event);
> +  Write_rows_log_event_old(const char *buf, uint event_len,
> +                           const Format_description_log_event *description_event);
>  #endif
>  #if !defined(MYSQL_CLIENT) 
>    static bool binlog_row_logging_function(THD *thd, TABLE *table,
> @@ -360,7 +355,7 @@ public:
>  #endif
>  
>  private:
> -  virtual Log_event_type get_type_code() { return (Log_event_type)TYPE_CODE; }
> +  virtual Log_event_type xget_type_code() { return (Log_event_type)XTYPE_CODE; }
>  
>  #ifdef MYSQL_CLIENT
>    void print(FILE *file, PRINT_EVENT_INFO *print_event_info);
> @@ -380,21 +375,6 @@ public:
>      TYPE_CODE = PRE_GA_WRITE_ROWS_EVENT
>    };
>  
> -#if !defined(MYSQL_CLIENT)
> -  Write_rows_log_event_old(THD *thd, TABLE *table, ulong table_id,
> -                           MY_BITMAP const *cols, bool is_transactional)
> -    : Write_rows_log_event(thd, table, table_id, cols, is_transactional)
> -  {
> -  }
> -#endif
> -#if defined(HAVE_REPLICATION)
> -  Write_rows_log_event_old(const char *buf, uint event_len,
> -                           const Format_description_log_event *descr)
> -    : Write_rows_log_event(buf, event_len, descr)
> -  {
> -  }
> -#endif
> -
>  private:
>    virtual Log_event_type get_type_code() { return (Log_event_type)TYPE_CODE; }
>  
> @@ -414,35 +394,29 @@ private:
>  };
>  
>  
> -class Update_rows_log_event_old 
> -  : public Update_rows_log_event, public Old_rows_log_event
> +class Update_rows_log_event_old : public Old_rows_log_event
>  {
>    /********** BEGIN CUT & PASTE FROM Update_rows_log_event **********/
>  public:
>    enum 
>    {
>      /* Support interface to THD::binlog_prepare_pending_rows_event */
> -    TYPE_CODE = UPDATE_ROWS_EVENT
> +    XTYPE_CODE = UPDATE_ROWS_EVENT
>   

See above.

>    };
>  
>  #ifndef MYSQL_CLIENT
> -  Update_rows_log_event(THD*, TABLE*, ulong table_id,
> -			MY_BITMAP const *cols_bi,
> -			MY_BITMAP const *cols_ai,
> -                        bool is_transactional);
> -
> -  Update_rows_log_event(THD*, TABLE*, ulong table_id,
> -			MY_BITMAP const *cols,
> -                        bool is_transactional);
> +  Update_rows_log_event_old(THD*, TABLE*, ulong table_id,
> +                            MY_BITMAP const *cols,
> +                            bool is_transactional);
>  
>    void init(MY_BITMAP const *cols);
>  #endif
>  
> -  virtual ~Update_rows_log_event();
> +  virtual ~Update_rows_log_event_old();
>  
>  #ifdef HAVE_REPLICATION
> -  Update_rows_log_event(const char *buf, uint event_len, 
> -			const Format_description_log_event *description_event);
> +  Update_rows_log_event_old(const char *buf, uint event_len,
> +                            const Format_description_log_event *description_event);
>  #endif
>  
>  #if !defined(MYSQL_CLIENT) 
> @@ -460,11 +434,11 @@ public:
>  
>    virtual bool is_valid() const
>    {
> -    return Rows_log_event::is_valid() && m_cols_ai.bitmap;
> +    return Old_rows_log_event::is_valid() && m_cols_ai.bitmap;
>    }
>  
>  protected:
> -  virtual Log_event_type get_type_code() { return (Log_event_type)TYPE_CODE; }
> +  virtual Log_event_type xget_type_code() { return (Log_event_type)XTYPE_CODE; }
>  
>  #ifdef MYSQL_CLIENT
>    void print(FILE *file, PRINT_EVENT_INFO *print_event_info);
> @@ -486,23 +460,6 @@ public:
>      TYPE_CODE = PRE_GA_UPDATE_ROWS_EVENT
>    };
>  
> -#if !defined(MYSQL_CLIENT)
> -  Update_rows_log_event_old(THD *thd, TABLE *table, ulong table_id,
> -                           MY_BITMAP const *cols, bool is_transactional)
> -    : Update_rows_log_event(thd, table, table_id, cols, is_transactional),
> -      m_after_image(NULL), m_memory(NULL)
> -  {
> -  }
> -#endif
> -#if defined(HAVE_REPLICATION)
> -  Update_rows_log_event_old(const char *buf, uint event_len, 
> -                            const Format_description_log_event *descr)
> -    : Update_rows_log_event(buf, event_len, descr),
> -      m_after_image(NULL), m_memory(NULL)
> -  {
> -  }
> -#endif
> -
>  private:
>    virtual Log_event_type get_type_code() { return (Log_event_type)TYPE_CODE; }
>  
> @@ -521,24 +478,23 @@ private:
>  };
>  
>  
> -class Delete_rows_log_event_old 
> -  : public Delete_rows_log_event, public Old_rows_log_event
> +class Delete_rows_log_event_old : public Old_rows_log_event
>  {
>    /********** BEGIN CUT & PASTE FROM Update_rows_log_event **********/
>  public:
>    enum 
>    {
>      /* Support interface to THD::binlog_prepare_pending_rows_event */
> -    TYPE_CODE = DELETE_ROWS_EVENT
> +    XTYPE_CODE = DELETE_ROWS_EVENT
>   

See above.

>    };
>  
>  #ifndef MYSQL_CLIENT
> -  Delete_rows_log_event(THD*, TABLE*, ulong, 
> -			MY_BITMAP const *cols, bool is_transactional);
> +  Delete_rows_log_event_old(THD*, TABLE*, ulong,
> +                            MY_BITMAP const *cols, bool is_transactional);
>  #endif
>  #ifdef HAVE_REPLICATION
> -  Delete_rows_log_event(const char *buf, uint event_len, 
> -			const Format_description_log_event *description_event);
> +  Delete_rows_log_event_old(const char *buf, uint event_len,
> +                            const Format_description_log_event *description_event);
>  #endif
>  #if !defined(MYSQL_CLIENT) 
>    static bool binlog_row_logging_function(THD *thd, TABLE *table,
> @@ -555,7 +511,7 @@ public:
>  #endif
>    
>  protected:
> -  virtual Log_event_type get_type_code() { return (Log_event_type)TYPE_CODE; }
> +  virtual Log_event_type xget_type_code() { return (Log_event_type)XTYPE_CODE; }
>   

The subclasses are now not proper drop-in replacements of the super 
classes, so this actually cause the inheritance hierarchy to break.

>  
>  #ifdef MYSQL_CLIENT
>    void print(FILE *file, PRINT_EVENT_INFO *print_event_info);
> @@ -566,7 +522,7 @@ protected:
>    virtual int do_after_row_operations(const Slave_reporting_capability *const,int);
>    virtual int do_exec_row(const Relay_log_info *const);
>  #endif
> -  /********** BEGIN CUT & PASTE FROM Update_rows_log_event **********/
> +  /********** END CUT & PASTE FROM Delete_rows_log_event **********/
>  
>    uchar *m_after_image, *m_memory;
>   
> @@ -576,23 +532,6 @@ public:
>      /* Support interface to THD::binlog_prepare_pending_rows_event */
>      TYPE_CODE = PRE_GA_DELETE_ROWS_EVENT
>    };
> -
> -#if !defined(MYSQL_CLIENT)
> -  Delete_rows_log_event_old(THD *thd, TABLE *table, ulong table_id,
> -                           MY_BITMAP const *cols, bool is_transactional)
> -    : Delete_rows_log_event(thd, table, table_id, cols, is_transactional),
> -      m_after_image(NULL), m_memory(NULL)
> -  {
> -  }
> -#endif
> -#if defined(HAVE_REPLICATION)
> -  Delete_rows_log_event_old(const char *buf, uint event_len,
> -                            const Format_description_log_event *descr)
> -    : Delete_rows_log_event(buf, event_len, descr),
> -      m_after_image(NULL), m_memory(NULL)
> -  {
> -  }
> -#endif
>  
>  private:
>    virtual Log_event_type get_type_code() { return (Log_event_type)TYPE_CODE; }
>
>   


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com