Fix for BUG#2983 "If statement was killed on master, slave errors despite replicate-wild-ignore-t"

We introduce a new function mysql_test_parse_for_slave().
If the slave sees that the query got a really bad error on master
(killed e.g.), then it calls this function to know if this query
can be ignored because of replicate-*-table rules (do not worry
about replicate-*-db rules: they are checked so early that they have
no bug). If the answer is yes, it skips the query and continues. If
it's no, then it stops and say "fix your slave data manually" (like it
did before this change).


mysql-test/r/rpl_error_ignored_table.result:
  result update
mysql-test/t/rpl_error_ignored_table-slave.opt:
  ignore more tables
mysql-test/t/rpl_error_ignored_table.test:
  we test if a killed query on the master, is ignored on the slave
  if the tables it involves should be excluded because of replicate-*-table
  rules.
sql/log_event.cc:
  If the query got a really bad error on the master (thread killed etc),
  parse it to test the table names: if the replicate-*-do|ignore-table
  rules say "this query must be ignored" then we exit gracefully;
  otherwise we warn about the bad error and tell DBA to check/fix it.
  Before this change, we always warned and stopped.
sql/mysql_priv.h:
  new function
sql/slave.cc:
  don't print error immediately as we need to do one more test to
  be sure.
sql/sql_parse.cc:
  we add a function mysql_test_parse_for_slave(), to be used only
  by the slave if it wants to know if the query should be ignored or not;
  so this function only parses the query, does not execute it.

4 files changed, 126 insertions, 84 deletions

diff --git a/sql/log_event.cc b/sql/log_event.cc
index d2957165e77..a484123b4c7 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -53,6 +53,14 @@ static void pretty_print_str(FILE* file, char* str, int len)
 
 #ifndef MYSQL_CLIENT
 
+static void clear_all_errors(THD *thd, struct st_relay_log_info *rli)
+{
+  thd->query_error = 0;
+  thd->clear_error();
+  *rli->last_slave_error = 0;
+  rli->last_slave_errno = 0;
+}
+
 inline int ignored_error_code(int err_code)
 {
   return ((err_code == ER_SLAVE_IGNORED_TABLE) ||
@@ -1803,8 +1811,7 @@ int Query_log_event::exec_event(struct st_relay_log_info* rli)
 #else
   rli->future_group_master_log_pos= log_pos;
 #endif
-  thd->query_error= 0;			// clear error
-  thd->clear_error();
+  clear_all_errors(thd, rli);
 
   if (db_ok(thd->db, replicate_do_db, replicate_ignore_db))
   {
@@ -1817,84 +1824,93 @@ int Query_log_event::exec_event(struct st_relay_log_info* rli)
     VOID(pthread_mutex_unlock(&LOCK_thread_count));
     thd->slave_proxy_id = thread_id;		// for temp tables
 	
-    /*
-      Sanity check to make sure the master did not get a really bad
-      error on the query.
-    */
-    if (ignored_error_code((expected_error = error_code)) ||
-	!check_expected_error(thd,rli,expected_error))
-    {
-      mysql_log.write(thd,COM_QUERY,"%s",thd->query);
-      DBUG_PRINT("query",("%s",thd->query));
+    mysql_log.write(thd,COM_QUERY,"%s",thd->query);
+    DBUG_PRINT("query",("%s",thd->query));
+    if (ignored_error_code(expected_error = error_code) ||
+        !check_expected_error(thd,rli,expected_error))
       mysql_parse(thd, thd->query, q_len);
-
-      /*
-	Set a flag if we are inside an transaction so that we can restart
-	the transaction from the start if we are killed
-
-	This will only be done if we are supporting transactional tables
-	in the slave.
-      */
-      if (!strcmp(thd->query,"BEGIN"))
-	rli->inside_transaction= opt_using_transactions;
-      else if (!(strcmp(thd->query,"COMMIT") && strcmp(thd->query,"ROLLBACK")))
-	rli->inside_transaction=0;
-
+    else
+    {
       /*
-        If we expected a non-zero error code, and we don't get the same error
-        code, and none of them should be ignored.
+        The query got a really bad error on the master (thread killed etc),
+        which could be inconsistent. Parse it to test the table names: if the
+        replicate-*-do|ignore-table rules say "this query must be ignored" then
+        we exit gracefully; otherwise we warn about the bad error and tell DBA
+        to check/fix it.
       */
-      if ((expected_error != (actual_error = thd->net.last_errno)) &&
-	  expected_error &&
-	  !ignored_error_code(actual_error) &&
-	  !ignored_error_code(expected_error))
+      if (mysql_test_parse_for_slave(thd, thd->query, q_len))
+        /* Can ignore query */
+        clear_all_errors(thd, rli);
+      else
       {
-	slave_print_error(rli, 0,
-                          "\
+        slave_print_error(rli,expected_error, 
+                          "query '%s' partially completed on the master \
+(error on master: %d) \
+and was aborted. There is a chance that your master is inconsistent at this \
+point. If you are sure that your master is ok, run this query manually on the\
+ slave and then restart the slave with SET GLOBAL SQL_SLAVE_SKIP_COUNTER=1;\
+ START SLAVE; .", thd->query, expected_error);
+        thd->query_error= 1;
+      }
+      goto end;
+    }
+
+    /*
+      Set a flag if we are inside an transaction so that we can restart
+      the transaction from the start if we are killed
+      
+      This will only be done if we are supporting transactional tables
+      in the slave.
+    */
+    if (!strcmp(thd->query,"BEGIN"))
+      rli->inside_transaction= opt_using_transactions;
+    else if (!(strcmp(thd->query,"COMMIT") && strcmp(thd->query,"ROLLBACK")))
+      rli->inside_transaction=0;
+    
+    /*
+      If we expected a non-zero error code, and we don't get the same error
+      code, and none of them should be ignored.
+    */
+    if ((expected_error != (actual_error = thd->net.last_errno)) &&
+        expected_error &&
+        !ignored_error_code(actual_error) &&
+        !ignored_error_code(expected_error))
+    {
+      slave_print_error(rli, 0,
+                        "\
 Query '%s' caused different errors on master and slave. \
 Error on master: '%s' (%d), Error on slave: '%s' (%d). \
 Default database: '%s'",
-                          query,
-                          ER_SAFE(expected_error),
-                          expected_error,
-                          actual_error ? thd->net.last_error: "no error",
-                          actual_error,
-                          print_slave_db_safe(db));
-	thd->query_error= 1;
-      }
-      /*
-        If we get the same error code as expected, or they should be ignored. 
-      */
-      else if (expected_error == actual_error ||
-	       ignored_error_code(actual_error))
-      {
-	thd->query_error = 0;
-        thd->clear_error();
-	*rli->last_slave_error = 0;
-	rli->last_slave_errno = 0;
-      }
-      /*
-        Other cases: mostly we expected no error and get one.
-      */
-      else if (thd->query_error || thd->fatal_error)
-      {
-        slave_print_error(rli,actual_error,
-			  "Error '%s' on query '%s'. Default database: '%s'",
-                          (actual_error ? thd->net.last_error :
-			   "unexpected success or fatal error"),
-			  query,
-                          print_slave_db_safe(db));
-        thd->query_error= 1;
-      }
-    } 
-    /* 
-       End of sanity check. If the test was wrong, the query got a really bad
-       error on the master, which could be inconsistent, abort and tell DBA to
-       check/fix it. check_expected_error() already printed the message to
-       stderr and rli, and set thd->query_error to 1.
+                        query,
+                        ER_SAFE(expected_error),
+                        expected_error,
+                        actual_error ? thd->net.last_error: "no error",
+                        actual_error,
+                        print_slave_db_safe(db));
+      thd->query_error= 1;
+    }
+    /*
+      If we get the same error code as expected, or they should be ignored. 
+    */
+    else if (expected_error == actual_error ||
+             ignored_error_code(actual_error))
+      clear_all_errors(thd, rli);
+    /*
+      Other cases: mostly we expected no error and get one.
     */
+    else if (thd->query_error || thd->fatal_error)
+    {
+      slave_print_error(rli,actual_error,
+                        "Error '%s' on query '%s'. Default database: '%s'",
+                        (actual_error ? thd->net.last_error :
+                         "unexpected success or fatal error"),
+                        query,
+                        print_slave_db_safe(db));
+      thd->query_error= 1;
+    }
   } /* End of if (db_ok(... */
 
+end:
   VOID(pthread_mutex_lock(&LOCK_thread_count));
   thd->db= 0;	                        // prevent db from being freed
   thd->query= 0;			// just to be sure
@@ -1939,8 +1955,7 @@ int Load_log_event::exec_event(NET* net, struct st_relay_log_info* rli,
   thd->db= (char*) rewrite_db(db);
   DBUG_ASSERT(thd->query == 0);
   thd->query = 0;				// Should not be needed
-  thd->query_error = 0;
-  thd->clear_error();
+  clear_all_errors(thd, rli);
 
   if (!use_rli_only_for_errors)
   {
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index 091e51629f0..3267b42effc 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -353,6 +353,7 @@ int quick_rm_table(enum db_type base,const char *db,
 bool mysql_rename_tables(THD *thd, TABLE_LIST *table_list);
 bool mysql_change_db(THD *thd,const char *name);
 void mysql_parse(THD *thd,char *inBuf,uint length);
+bool mysql_test_parse_for_slave(THD *thd,char *inBuf,uint length);
 void mysql_init_select(LEX *lex);
 bool mysql_new_select(LEX *lex);
 void mysql_init_multi_delete(LEX *lex);
diff --git a/sql/slave.cc b/sql/slave.cc
index 30052e874b3..d6dda473be0 100644
--- a/sql/slave.cc
+++ b/sql/slave.cc
@@ -2250,13 +2250,6 @@ int check_expected_error(THD* thd, RELAY_LOG_INFO* rli, int expected_error)
   case ER_NET_ERROR_ON_WRITE:  
   case ER_SERVER_SHUTDOWN:  
   case ER_NEW_ABORTING_CONNECTION:
-    slave_print_error(rli,expected_error, 
-                      "query '%s' partially completed on the master \
-and was aborted. There is a chance that your master is inconsistent at this \
-point. If you are sure that your master is ok, run this query manually on the\
- slave and then restart the slave with SET GLOBAL SQL_SLAVE_SKIP_COUNTER=1;\
- SLAVE START; .", thd->query);
-    thd->query_error= 1;
     return 1;
   default:
     return 0;
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 88af18491ec..5b1487aa313 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -65,6 +65,7 @@ static bool create_total_list(THD *thd, LEX *lex,
 			      TABLE_LIST **result, bool skip_first);
 static bool check_one_table_access(THD *thd, ulong want_access,
 				   TABLE_LIST *table, bool no_errors);
+static inline bool all_tables_not_ok(THD *thd, TABLE_LIST *tables);
 
 const char *any_db="*any*";	// Special symbol for check_access
 
@@ -1332,9 +1333,7 @@ mysql_execute_command(void)
       Skip if we are in the slave thread, some table rules have been
       given and the table list says the query should not be replicated
     */
-    if (table_rules_on && tables && !tables_ok(thd,tables) &&
-        ((lex->sql_command != SQLCOM_DELETE_MULTI) ||
-         !tables_ok(thd,(TABLE_LIST *)thd->lex.auxilliary_table_list.first)))
+    if (all_tables_not_ok(thd,tables))
     {
       /* we warn the slave SQL thread */
       my_error(ER_SLAVE_IGNORED_TABLE, MYF(0));
@@ -2968,9 +2967,18 @@ void mysql_init_multi_delete(LEX *lex)
   lex->select->table_list.save_and_clear(&lex->auxilliary_table_list);
 }
 
+static inline bool all_tables_not_ok(THD *thd, TABLE_LIST *tables)
+{
+  return (table_rules_on && tables && !tables_ok(thd,tables) &&
+          ((thd->lex.sql_command != SQLCOM_DELETE_MULTI) ||
+           !tables_ok(thd,(TABLE_LIST *)thd->lex.auxilliary_table_list.first)));
+}
 
-void
-mysql_parse(THD *thd,char *inBuf,uint length)
+/*
+  When you modify mysql_parse(), you may need to mofify
+  mysql_test_parse_for_slave() in this same file.
+*/
+void mysql_parse(THD *thd, char *inBuf, uint length)
 {
   DBUG_ENTER("mysql_parse");
 
@@ -3005,6 +3013,31 @@ mysql_parse(THD *thd,char *inBuf,uint length)
   DBUG_VOID_RETURN;
 }
 
+/*
+  Usable by the replication SQL thread only: just parse a query to know if it
+  can be ignored because of replicate-*-table rules.
+
+  RETURN VALUES
+    0	cannot be ignored
+    1	can be ignored
+*/
+
+bool mysql_test_parse_for_slave(THD *thd, char *inBuf, uint length)
+{
+  LEX *lex;
+  bool error= 0;
+
+  mysql_init_query(thd);
+  lex= lex_start(thd, (uchar*) inBuf, length);
+  if (!yyparse() && ! thd->fatal_error &&
+      all_tables_not_ok(thd,(TABLE_LIST*) lex->select_lex.table_list.first))
+    error= 1;                /* Ignore question */
+  free_items(thd);  /* Free strings used by items */
+  lex_end(lex);
+
+  return error;
+}
+
 
 /*****************************************************************************
 ** Store field definition for create