MySQL Bugs: #51021: current_stmt_binlog_row_based not removed in next-mr

Bug #51021	current_stmt_binlog_row_based not removed in next-mr
Submitted:	9 Feb 2010 11:21	Modified:	8 Mar 2010 20:27
Reporter:	Sven Sandberg	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Replication	Severity:	S2 (Serious)
Version:	next-mr, 5.1-rpl-merge	OS:	Any
Assigned to:	Luis Soares	CPU Architecture:	Any

Description:
In 5.1-rep+3, particularly the fix to BUG#39934, the class variable THD::current_stmt_binlog_row_based was removed. Instead, we now have the class functions THD::is_current_stmt_binlog_format_row(), THD::set_current_stmt_binlog_format_row(), and THD::clear_current_stmt_binlog_format_row(). These class functions should be used exclusively.

In next-mr and 5.1-rpl-merge, THD::current_stmt_binlog_row_based is used again, even after 5.1-rep+3 has been merged to it.

This bug is urgent as it holds a merge from 5.1 to 5.1-rep+2

How to repeat:
Read the code.

Suggested fix:
1. Figure out which merge is the bad one, check if anything else went wrong.

2. Figure out why the error was not caught by tests. It may be the case that we have bad coverage, or that a result file was updated incorrectly.

3. Remove THD::current_stmt_binlog_row_based. In places where it is used, replace it by appropriate class function calls.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/99760

3002 Luis Soares	2010-02-09
      BUG#51021: current_stmt_binlog_row_based not removed in next-mr
      
      As part of BUG@39934 fix, the public:
       - THD::current_stmt_binlog_row_based 
      variable had been removed and replaced by a private variable:
       - THD::current_stmt_binlog_format. 
      
      THD was refactored and some modifiers and accessors were
      implemented for the new variable.
      
      However, due to a bad merge, the
      THD::current_stmt_binlog_row_based variable is back as a public
      member of THD. This in itself is already potentially
      harmful. What's even worse is that while merging some more
      patches and resolving conflicts, the variable started being used
      again, which is obviously wrong.
      
      To fix this we:
        1. remove the extraneous variable from sql_class.h
        2. revert a bad merge for BUG#49132
        3. merge BUG#49132 properly again (actually, making use of the
           cset used to merge the original patch to mysql-pe).

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/99847

3003 Luis Soares	2010-02-10
      BUG#51021: current_stmt_binlog_row_based not removed in next-mr
      
      Deployed DBUG_ASSERT before the conditional binlog format
      restore.

A closely related problem, hardly worth a new bug report: there is an extra thd->set_current_stmt_binlog_format_row_if_mixed() call in sql_base.cc:lock_tables(). This was meant to be removed in the fix of BUG#39934, but I forgot to do it. I think we can remove it as part of this bugfix.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/99882

3004 Luis Soares	2010-02-10
      BUG#51021: current_stmt_binlog_row_based not removed in next-mr
      
      A closely related problem, hardly worth a new bug report:
      Removed a spurious call to:  
        thd->set_current_stmt_binlog_format_row_if_mixed()
      in sql_base.cc:lock_tables().

Queued in:
- mysql-next-mr-bugfixing
  http://pb2.norway.sun.com/web.py?template=push_details&push=947928
- mysql-6.0-codebase-bugfixing
  http://pb2.norway.sun.com/web.py?template=push_details&push=947924

Pushed into 6.0.14-alpha (revid:alik@sun.com-20100213083436-9pesg4h55w1mekxc) (version source revid:luis.soares@sun.com-20100211135109-t63avry9fqpgyh78) (merge vers: 6.0.14-alpha) (pib:16)

Pushed into mysql-next-mr (revid:alik@sun.com-20100213083327-cee4ao3jpg33eggv) (version source revid:luis.soares@sun.com-20100211135018-1f9dbghg0itszigo) (pib:16)

Hi!

I realise this is a very important fix, but I'm not sure if there's any *direct* effect on users.

Can you describe what this effect might be, in a sentence or two?

Otherwise, you can close this bug report if the fix has already been merged to all necessary trees.

Thanks!

jon.

Documented in the 6.0.14 changelog as follows:

      An issue internal to the code, first seen in Bug#49132 but not 
      completely resolved in the fix for that bug, was removed. This 
      should prevent similar issues to those in the previous bug with 
      binlog_format changes following DDL statements. 

      More precisely, the class variable 
      THD::current_stmt_binlog_row_based was supposed to have been 
      removed as part of the fix for Bug#39934 but was still present 
      in the MySQL Server code. If a developer later tried to use this
      variable, it could cause the previous issues to re-occur, and 
      possibly new ones to arise. The variable has now been removed; 
      the previously added class functions 
      THD::is_current_stmt_binlog_format_row(),
      THD::set_current_stmt_binlog_format_row(), and
      THD::clear_current_stmt_binlog_format_row() should be used 
      instead.

Setting status Need Merge -- looks like we're waiting on merges to 5.1 and 5.5?

(Let me know if this is not the case.)

Luis confirms that this isn't slated for any more merges. Closing.

Pushed into 5.5.3-m3 (revid:alik@sun.com-20100306103849-hha31z2enhh7jwt3) (version source revid:vvaintroub@mysql.com-20100213160132-nx1vlocxuta76txh) (merge vers: 5.5.99-m3) (pib:16)

Also documented in the 5.5.3 changelog.

Closed.