Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3117) Bug#46640

[Libing Song]

Hi Daogang:
Nice work.
Patch approved.

Kristian Nielsen wrote:
> Libing Song <Li-Bing.Song@stripped> writes:
>
>   
>> Dao-Gang.Qu@stripped wrote:
>>     
>
>   
>>>  @@ -7621,33 +7625,22 @@ Rows_log_event::do_update_pos(Relay_log_
>>>     if (get_flags(STMT_END_F))
>>>    {
>>> -    if ((error= rows_event_stmt_cleanup(rli, thd)) == 0)
>>> -    {
>>> -      /*
>>> -        Indicate that a statement is finished.
>>> -        Step the group log position if we are not in a transaction,
>>> -        otherwise increase the event log position.
>>> -      */
>>> -      rli->stmt_done(log_pos, when);
>>> -
>>> -      /*
>>> -        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
>>> -        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.
>>> -      */
>>> -      thd->clear_error();
>>> -    }
>>> -    else
>>> -    {
>>> -      rli->report(ERROR_LEVEL, error,
>>> -                  "Error in %s event: commit of row events failed, "
>>> -                  "table `%s`.`%s`",
>>> -                  get_type_str(), m_table->s->db.str,
>>> -                  m_table->s->table_name.str);
>>> -    }
>>> +    /*
>>> +      Indicate that a statement is finished.
>>> +      Step the group log position if we are not in a transaction,
>>> +      otherwise increase the event log position.
>>> +    */
>>> +    rli->stmt_done(log_pos, when);
>>> +    +    /*
>>> +      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
>>> +      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.
>>> +    */
>>> +    thd->clear_error();
>>>    }
>>>    else
>>>    {
>>>
>>>   
>>>       
>> In my mind,  Above code only moves "rows_event_stmt_cleanup" from
>> do_update_pos
>> to do_apply_event.
>> I don't know why above code is need.
>> I removed above code and I found the test case works well yet.
>>     
>
> Yes, the purpose is to move rows_event_stmt_cleanup() from do_update_pos() to
> do_apply_event().
>
> The reason is that the patch changes BINLOG statement to not call
> apply_event_and_update_pos() (and hence do_update_pos()), only
> do_apply_event(). But rows_event_stmt_cleanup() does some actions that are
> needed, eg. at end of transaction.
>
> And the reason for not calling apply_event_and_update_pos() is that it does a
> number of things that should not happen in BINLOG, in particular setting
> server_id.
>
> Check also comments by Sven Sandberg in
> http://bugs.mysql.com/bug.php?id=46640.
>
> Thanks,
>
>  - Kristian.
>
>