2 minor edits, plus

fix for BUG#1113 "INSERT into non-trans table SELECT ; ROLLBACK" does not send warning"
and
fix for BUG#873 "In transaction, INSERT to non-trans table is written too early to binlog".
Now we don't always write the non-trans update immediately to the binlog;
if there is something in the binlog cache we write it to the binlog cache
(because the non-trans update could depend on a trans table which was modified
earlier in the transaction); then in case of ROLLBACK, we write the binlog
cache to the binlog, wrapped with BEGIN/ROLLBACK.
This guarantees that the slave does the same updates.
For ROLLBACK TO SAVEPOINT: when we execute a SAVEPOINT command we write it
to the binlog cache. At ROLLBACK TO SAVEPOINT, if some non-trans table was updated,
we write ROLLBACK TO SAVEPOINT to the binlog cache; when the transaction
terminates (COMMIT/ROLLBACK), the binlog cache will be flushed to the binlog
(because of the non-trans update) so we'll have SAVEPOINT and ROLLBACK TO
SAVEPOINT in the binlog.

Apart from this rare case of updates of mixed table types in transaction, the
usual way is still clear the binlog cache at ROLLBACK, or chop it at
ROLLBACK TO SAVEPOINT (meaning the SAVEPOINT command is also chopped, which
is fine).
Note that BUG#873 encompasses subbugs 1) and 2) of BUG#333 "3 binlogging bugs when doing INSERT with mixed InnoDB/MyISAM".


client/mysqldump.c:
  Minor edit: one CHANGE MASTER with 2 arguments instead of 2 CHANGE MASTER with one argument each.
mysql-test/r/rpl_loaddata.result:
  result update
mysql-test/t/rpl_loaddata.test:
  minor edit: simplifying the test.
sql/handler.cc:
  Fix for BUG#873. See comments in code, and the description of the changeset.
sql/log.cc:
  * Previously, if a query updated a non-transactional table we wrote it immediately
  to the real binlog. This causes a bug when the update is done inside a transaction
  and uses the content of an updated transactional table (because this makes
  a wrong order of queries in the binlog). So if the binlog cache is not empty,
  we write the query to the binlog cache; otherwise we can write it to the binlog.
  * Previously, when we flushed the binlog cache to the binlog, we wrapped it
  with BEGIN/COMMIT. Now it's also possible to wrap it with BEGIN/ROLLBACK, to handle
  transactions which update both transactional and non-transactional tables.
sql/log_event.cc:
  The slave thread can leave a transaction if COMMIT or if ROLLBACK.
sql/sql_class.h:
  prototype
sql/sql_insert.cc:
  Fix for BUG#1113:
  this was because the INSERT SELECT code did not set OPTION_STATUS_NO_TRANS_UPDATE.
sql/sql_parse.cc:
  Don't send ER_WARNING_NOT_COMPLETE_ROLLBACK if this is the SQL slave thread (see comments).

6 files changed, 105 insertions, 16 deletions

diff --git a/sql/handler.cc b/sql/handler.cc
index 96611301bfa..0d29dbeaa31 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -349,7 +349,7 @@ int ha_commit_trans(THD *thd, THD_TRANS* trans)
     if (trans == &thd->transaction.all && mysql_bin_log.is_open() &&
 	my_b_tell(&thd->transaction.trans_log))
     {
-      mysql_bin_log.write(thd, &thd->transaction.trans_log);
+      mysql_bin_log.write(thd, &thd->transaction.trans_log, 1);
       reinit_io_cache(&thd->transaction.trans_log,
 		      WRITE_CACHE, (my_off_t) 0, 0, 1);
       thd->transaction.trans_log.end_of_file= max_binlog_cache_size;
@@ -432,9 +432,21 @@ int ha_rollback_trans(THD *thd, THD_TRANS *trans)
     }
 #endif
     if (trans == &thd->transaction.all)
+    {
+      /* 
+         Update the binary log with a BEGIN/ROLLBACK block if we have cached some
+         queries and we updated some non-transactional table. Such cases should
+         be rare (updating a non-transactional table inside a transaction...).
+      */
+      if (unlikely((thd->options & OPTION_STATUS_NO_TRANS_UPDATE) &&
+                   mysql_bin_log.is_open() &&
+                   my_b_tell(&thd->transaction.trans_log)))
+        mysql_bin_log.write(thd, &thd->transaction.trans_log, 0);
+      /* Flushed or not, empty the binlog cache */
       reinit_io_cache(&thd->transaction.trans_log,
-		      WRITE_CACHE, (my_off_t) 0, 0, 1);
-    thd->transaction.trans_log.end_of_file= max_binlog_cache_size;
+                      WRITE_CACHE, (my_off_t) 0, 0, 1);
+      thd->transaction.trans_log.end_of_file= max_binlog_cache_size;
+    }
     thd->variables.tx_isolation=thd->session_tx_isolation;
     if (operation_done)
     {
@@ -448,9 +460,27 @@ int ha_rollback_trans(THD *thd, THD_TRANS *trans)
 
 
 /*
-Rolls the current transaction back to a savepoint.
-Return value: 0 if success, 1 if there was not a savepoint of the given
-name.
+  Rolls the current transaction back to a savepoint.
+  Return value: 0 if success, 1 if there was not a savepoint of the given
+  name.
+  NOTE: how do we handle this (unlikely but legal) case:
+  [transaction] + [update to non-trans table] + [rollback to savepoint] ?
+  The problem occurs when a savepoint is before the update to the
+  non-transactional table. Then when there's a rollback to the savepoint, if we
+  simply truncate the binlog cache, we lose the part of the binlog cache where
+  the update is. If we want to not lose it, we need to write the SAVEPOINT
+  command and the ROLLBACK TO SAVEPOINT command to the binlog cache. The latter
+  is easy: it's just write at the end of the binlog cache, but the former should
+  be *inserted* to the place where the user called SAVEPOINT. The solution is
+  that when the user calls SAVEPOINT, we write it to the binlog cache (so no
+  need to later insert it). As transactions are never intermixed in the binary log
+  (i.e. they are serialized), we won't have conflicts with savepoint names when
+  using mysqlbinlog or in the slave SQL thread.
+  Then when ROLLBACK TO SAVEPOINT is called, if we updated some
+  non-transactional table, we don't truncate the binlog cache but instead write
+  ROLLBACK TO SAVEPOINT to it; otherwise we truncate the binlog cache (which
+  will chop the SAVEPOINT command from the binlog cache, which is good as in
+  that case there is no need to have it in the binlog).
 */
 
 int ha_rollback_to_savepoint(THD *thd, char *savepoint_name)
@@ -475,8 +505,24 @@ int ha_rollback_to_savepoint(THD *thd, char *savepoint_name)
       error=1;
     }
     else
-      reinit_io_cache(&thd->transaction.trans_log, WRITE_CACHE,
-						 binlog_cache_pos, 0, 0);
+    {
+      /* 
+         Write ROLLBACK TO SAVEPOINT to the binlog cache if we have updated some
+         non-transactional table. Otherwise, truncate the binlog cache starting
+         from the SAVEPOINT command.
+      */
+      if (unlikely((thd->options & OPTION_STATUS_NO_TRANS_UPDATE) &&
+                   mysql_bin_log.is_open() &&
+                   my_b_tell(&thd->transaction.trans_log)))
+      {
+        Query_log_event qinfo(thd, thd->query, thd->query_length, TRUE);
+        if (mysql_bin_log.write(&qinfo))
+          error= 1;
+      }
+      else
+        reinit_io_cache(&thd->transaction.trans_log, WRITE_CACHE,
+                        binlog_cache_pos, 0, 0);
+    }
     operation_done=1;
 #endif
     if (operation_done)
@@ -505,6 +551,13 @@ int ha_savepoint(THD *thd, char *savepoint_name)
 #ifdef HAVE_INNOBASE_DB
     innobase_savepoint(thd,savepoint_name, binlog_cache_pos);
 #endif
+    /* Write it to the binary log (see comments of ha_rollback_to_savepoint). */
+    if (mysql_bin_log.is_open())
+    {
+      Query_log_event qinfo(thd, thd->query, thd->query_length, TRUE);
+      if (mysql_bin_log.write(&qinfo))
+	error= 1;
+    }
   }
 #endif /* USING_TRANSACTIONS */
   DBUG_RETURN(error);
diff --git a/sql/log.cc b/sql/log.cc
index ee774ea3700..6e9fa38c407 100644
--- a/sql/log.cc
+++ b/sql/log.cc
@@ -1062,7 +1062,17 @@ bool MYSQL_LOG::write(Log_event* event_info)
     bool should_rotate = 0;
     const char *local_db = event_info->get_db();
 #ifdef USING_TRANSACTIONS    
-    IO_CACHE *file = ((event_info->get_cache_stmt()) ?
+    /*
+      Should we write to the binlog cache or to the binlog on disk?
+      Write to the binlog cache if:
+      - it is already not empty (meaning we're in a transaction; note that the
+     present event could be about a non-transactional table, but still we need
+     to write to the binlog cache in that case to handle updates to mixed
+     trans/non-trans table types the best possible in binlogging)
+      - or if the event asks for it (cache_stmt == true).
+    */
+    IO_CACHE *file = ((event_info->get_cache_stmt() ||
+                       my_b_tell(&thd->transaction.trans_log)) ? 
 		      &thd->transaction.trans_log :
 		      &log_file);
 #else
@@ -1258,6 +1268,13 @@ uint MYSQL_LOG::next_file_id()
 /*
   Write a cached log entry to the binary log
 
+  SYNOPSIS
+    write()
+    thd 		
+    cache		The cache to copy to the binlog
+    commit_or_rollback  If true, will write "COMMIT" in the end, if false will
+                        write "ROLLBACK".
+
   NOTE
     - We only come here if there is something in the cache.
     - The thing in the cache is always a complete transaction
@@ -1265,10 +1282,13 @@ uint MYSQL_LOG::next_file_id()
 
   IMPLEMENTATION
     - To support transaction over replication, we wrap the transaction
-      with BEGIN/COMMIT in the binary log.
+      with BEGIN/COMMIT or BEGIN/ROLLBACK in the binary log.
+      We want to write a BEGIN/ROLLBACK block when a non-transactional table was
+      updated in a transaction which was rolled back. This is to ensure that the
+      same updates are run on the slave.
 */
 
-bool MYSQL_LOG::write(THD *thd, IO_CACHE *cache)
+bool MYSQL_LOG::write(THD *thd, IO_CACHE *cache, bool commit_or_rollback)
 {
   VOID(pthread_mutex_lock(&LOCK_log));
   DBUG_ENTER("MYSQL_LOG::write(cache");
@@ -1322,7 +1342,10 @@ bool MYSQL_LOG::write(THD *thd, IO_CACHE *cache)
     */
 
     {
-      Query_log_event qinfo(thd, "COMMIT", 6, TRUE);
+      Query_log_event qinfo(thd, 
+                            commit_or_rollback ? "COMMIT" : "ROLLBACK",
+                            commit_or_rollback ? 6        : 8, 
+                            TRUE);
       qinfo.set_log_pos(this);
       if (qinfo.write(&log_file) || flush_io_cache(&log_file))
 	goto err;
diff --git a/sql/log_event.cc b/sql/log_event.cc
index 9e7546dd635..54e4d34f77e 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -1803,7 +1803,7 @@ int Query_log_event::exec_event(struct st_relay_log_info* rli)
       */
       if (!strcmp(thd->query,"BEGIN"))
 	rli->inside_transaction= opt_using_transactions;
-      else if (!strcmp(thd->query,"COMMIT"))
+      else if (!(strcmp(thd->query,"COMMIT") && strcmp(thd->query,"ROLLBACK")))
 	rli->inside_transaction=0;
 
       /*
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 49a364856eb..b30faa3e9d9 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -140,7 +140,7 @@ public:
   bool write(THD *thd, const char *query, uint query_length,
 	     time_t query_start=0);
   bool write(Log_event* event_info); // binary log write
-  bool write(THD *thd, IO_CACHE *cache);
+  bool write(THD *thd, IO_CACHE *cache, bool commit_or_rollback);
 
   /*
     v stands for vector
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index 167ccf974c7..0a83358e8c6 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -1367,6 +1367,8 @@ void select_insert::send_error(uint errcode,const char *err)
                             table->file->has_transactions());
       mysql_bin_log.write(&qinfo);
     }
+    if (!table->tmp_table)
+      thd->options|=OPTION_STATUS_NO_TRANS_UPDATE;    
   }
   ha_rollback_stmt(thd);
   if (info.copied || info.deleted)
@@ -1398,6 +1400,8 @@ bool select_insert::send_eof()
   if (info.copied || info.deleted)
   {
     query_cache_invalidate3(thd, table, 1);
+    if (!(table->file->has_transactions() || table->tmp_table))
+	thd->options|=OPTION_STATUS_NO_TRANS_UPDATE;
   }
   if (error)
   {
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 1479a611b5a..548714345f8 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -2553,7 +2553,16 @@ mysql_execute_command(void)
     thd->server_status&= ~SERVER_STATUS_IN_TRANS;
     if (!ha_rollback(thd))
     {
-      if (thd->options & OPTION_STATUS_NO_TRANS_UPDATE)
+      /*
+        If a non-transactional table was updated, warn; don't warn if this is a
+        slave thread (because when a slave thread executes a ROLLBACK, it has
+        been read from the binary log, so it's 100% sure and normal to produce
+        error ER_WARNING_NOT_COMPLETE_ROLLBACK. If we sent the warning to the
+        slave SQL thread, it would not stop the thread but just be printed in
+        the error log; but we don't want users to wonder why they have this
+        message in the error log, so we don't send it.
+      */
+      if ((thd->options & OPTION_STATUS_NO_TRANS_UPDATE) && !thd->slave_thread)
 	send_warning(&thd->net,ER_WARNING_NOT_COMPLETE_ROLLBACK,0);
       else
 	send_ok(&thd->net);
@@ -2565,7 +2574,7 @@ mysql_execute_command(void)
   case SQLCOM_ROLLBACK_TO_SAVEPOINT:
     if (!ha_rollback_to_savepoint(thd, lex->savepoint_name))
     {
-      if (thd->options & OPTION_STATUS_NO_TRANS_UPDATE)
+      if ((thd->options & OPTION_STATUS_NO_TRANS_UPDATE) && !thd->slave_thread)
 	send_warning(&thd->net,ER_WARNING_NOT_COMPLETE_ROLLBACK,0);
       else
 	send_ok(&thd->net);