From 388cf622eedda42762afa99ae48fb7b2287ffe18 Mon Sep 17 00:00:00 2001
From: "guilhem@mysql.com" <>
Date: Tue, 20 Sep 2005 17:41:47 +0200
Subject: [PATCH] Fix fur BUG#13348: "multi-table updates and deletes are not
 logged if no rows were affected". Not fixed in 4.1 as not critical. Also I'm
 correcting error checking of multi-UPDATE/DELETE when it comes to binlogging,
 to make it consistent with when we rollback the statement.

---
 mysql-test/r/rpl_multi_delete.result |  9 +++++++++
 mysql-test/r/rpl_multi_update.result | 13 +++++++++++++
 mysql-test/t/rpl_multi_delete.test   | 26 +++++++++++++++++++++-----
 mysql-test/t/rpl_multi_update.test   | 23 +++++++++++++++++++++++
 sql/sql_delete.cc                    | 28 ++++++++--------------------
 sql/sql_update.cc                    | 21 ++++++++++++++-------
 6 files changed, 88 insertions(+), 32 deletions(-)

diff --git a/mysql-test/r/rpl_multi_delete.result b/mysql-test/r/rpl_multi_delete.result
index e94a4e7947e..d2c68eee62e 100644
--- a/mysql-test/r/rpl_multi_delete.result
+++ b/mysql-test/r/rpl_multi_delete.result
@@ -19,4 +19,13 @@ a
 select * from t2;
 a
 1
+delete from t1;
+delete from t2;
+insert into t1 values(1);
+insert into t2 values(1);
+DELETE t1.*, t2.* from t1, t2;
+select * from t1;
+a
+select * from t2;
+a
 drop table t1,t2;
diff --git a/mysql-test/r/rpl_multi_update.result b/mysql-test/r/rpl_multi_update.result
index 34f99746c7d..04cb1bc7460 100644
--- a/mysql-test/r/rpl_multi_update.result
+++ b/mysql-test/r/rpl_multi_update.result
@@ -24,3 +24,16 @@ a	b
 1	0
 2	1
 UPDATE t1, t2 SET t1.b = t2.b WHERE t1.a = t2.a;
+delete from t1;
+delete from t2;
+insert into t1 values(1,1);
+insert into t2 values(1,1);
+update t1 set a=2;
+UPDATE t1, t2 SET t1.a = t2.a;
+select * from t1;
+a	b
+1	1
+select * from t2;
+a	b
+1	1
+drop table t1, t2;
diff --git a/mysql-test/t/rpl_multi_delete.test b/mysql-test/t/rpl_multi_delete.test
index 2fd7b820b1a..0538d0d7bae 100644
--- a/mysql-test/t/rpl_multi_delete.test
+++ b/mysql-test/t/rpl_multi_delete.test
@@ -16,10 +16,26 @@ sync_with_master;
 select * from t1;
 select * from t2;
 
+# End of 4.1 tests
+
+# Check if deleting 0 rows is binlogged (BUG#13348)
+
+connection master;
+delete from t1;
+delete from t2;
+
+connection slave;
+# force a difference to see if master's multi-DELETE will correct it
+insert into t1 values(1);
+insert into t2 values(1);
+
+connection master;
+DELETE t1.*, t2.* from t1, t2;
+
+sync_slave_with_master;
+select * from t1;
+select * from t2;
+
 connection master;
 drop table t1,t2;
-save_master_pos;
-connection slave;
-sync_with_master;
-
-# End of 4.1 tests
+sync_slave_with_master;
diff --git a/mysql-test/t/rpl_multi_update.test b/mysql-test/t/rpl_multi_update.test
index dd75edb3055..f400e722556 100644
--- a/mysql-test/t/rpl_multi_update.test
+++ b/mysql-test/t/rpl_multi_update.test
@@ -24,3 +24,26 @@ connection slave;
 sync_with_master;
 
 # End of 4.1 tests
+
+# Check if updating 0 rows is binlogged (BUG#13348)
+
+connection master;
+delete from t1;
+delete from t2;
+insert into t1 values(1,1);
+insert into t2 values(1,1);
+
+connection slave;
+# force a difference to see if master's multi-UPDATE will correct it
+update t1 set a=2;
+
+connection master;
+UPDATE t1, t2 SET t1.a = t2.a;
+
+sync_slave_with_master;
+select * from t1;
+select * from t2;
+
+connection master;
+drop table t1, t2;
+sync_slave_with_master;
diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc
index 7fb9f9eccdd..7e99a5c65ee 100644
--- a/sql/sql_delete.cc
+++ b/sql/sql_delete.cc
@@ -258,19 +258,12 @@ cleanup:
 
   delete select;
   transactional_table= table->file->has_transactions();
-  /*
-    We write to the binary log even if we deleted no row, because maybe the
-    user is using this command to ensure that a table is clean on master *and
-    on slave*. Think of the case of a user having played separately with the
-    master's table and slave's table and wanting to take a fresh identical
-    start now.
-    error < 0 means "really no error". error <= 0 means "maybe some error".
-  */
-  if ((deleted || (error < 0)) && (error <= 0 || !transactional_table))
+  /* See similar binlogging code in sql_update.cc, for comments */
+  if ((error < 0) || (deleted && !transactional_table))
   {
     if (mysql_bin_log.is_open())
     {
-      if (error <= 0)
+      if (error < 0)
         thd->clear_error();
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_table, FALSE);
@@ -718,6 +711,9 @@ bool multi_delete::send_eof()
   /* Does deletes for the last n - 1 tables, returns 0 if ok */
   int local_error= do_deletes();		// returns 0 if success
 
+  /* compute a total error to know if something failed */
+  local_error= local_error || error;
+
   /* reset used flags */
   thd->proc_info="end";
 
@@ -730,19 +726,11 @@ bool multi_delete::send_eof()
     query_cache_invalidate3(thd, delete_tables, 1);
   }
 
-  /*
-    Write the SQL statement to the binlog if we deleted
-    rows and we succeeded, or also in an error case when there
-    was a non-transaction-safe table involved, since
-    modifications in it cannot be rolled back.
-    Note that if we deleted nothing we don't write to the binlog (TODO:
-    fix this).
-  */
-  if (deleted && ((error <= 0 && !local_error) || normal_tables))
+  if ((local_error == 0) || (deleted && normal_tables))
   {
     if (mysql_bin_log.is_open())
     {
-      if (error <= 0 && !local_error)
+      if (local_error == 0)
         thd->clear_error();
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_tables, FALSE);
diff --git a/sql/sql_update.cc b/sql/sql_update.cc
index a75eef9324c..efdf1291305 100644
--- a/sql/sql_update.cc
+++ b/sql/sql_update.cc
@@ -475,11 +475,20 @@ int mysql_update(THD *thd,
     query_cache_invalidate3(thd, table_list, 1);
   }
 
-  if ((updated || (error < 0)) && (error <= 0 || !transactional_table))
+  /*
+    error < 0 means really no error at all: we processed all rows until the
+    last one without error. error > 0 means an error (e.g. unique key
+    violation and no IGNORE or REPLACE). error == 0 is also an error (if
+    preparing the record or invoking before triggers fails). See
+    ha_autocommit_or_rollback(error>=0) and DBUG_RETURN(error>=0) below.
+    Sometimes we want to binlog even if we updated no rows, in case user used
+    it to be sure master and slave are in same state.
+  */
+  if ((error < 0) || (updated && !transactional_table))
   {
     if (mysql_bin_log.is_open())
     {
-      if (error <= 0)
+      if (error < 0)
         thd->clear_error();
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_table, FALSE);
@@ -1439,16 +1448,14 @@ bool multi_update::send_eof()
   /*
     Write the SQL statement to the binlog if we updated
     rows and we succeeded or if we updated some non
-    transacational tables.
-    Note that if we updated nothing we don't write to the binlog (TODO:
-    fix this).
+    transactional tables.
   */
 
-  if (updated && (local_error <= 0 || !trans_safe))
+  if ((local_error == 0) || (updated && !trans_safe))
   {
     if (mysql_bin_log.is_open())
     {
-      if (local_error <= 0)
+      if (local_error == 0)
         thd->clear_error();
       Query_log_event qinfo(thd, thd->query, thd->query_length,
 			    transactional_tables, FALSE);