Fix SRO + multi-table update statements in replication when logged in…

… statement form.

Summary:
This was contributed as:
#11

This is a bug fix for super-read-only not being ignored for the replication thread on multi-table
update statements in sql_parse.cc that would cause replication the stop with the error
"Error 'The MySQL server is running with the --read-only (super) option so it cannot execute this statement'
on query. Default database: 'test'. Query: 'update t2 natural join t1 set b=3'".
The same issue was found in transaction.cc.

To help avoid this issue in the future the logic for testing if read-only should
be enforced is refactored into a new inline function, and existing code that
performed the logic in each location was changed to instead call this function.

Test Plan: mtr test

Reviewers: steaphan, jtolmer

Reviewed By: jtolmer

mysql-test/suite/rpl/r/rpl_ignore_super_read_only.result

@@ -1,5 +1,6 @@
 # This unit test is only to test super_read_only=1 doesn't cause
-# errors when executing "CHANGE MASTER".
+# errors when executing "CHANGE MASTER" or from statements executed
+# via replication.
 #
 # It is not intended to test general super_read_only functionality.
 #
@@ -12,18 +13,27 @@ Note	####	Storing MySQL user name or password information in the master info rep
 [connection master]
 create table t1(a int);
 insert into t1 values(1);
+create table t2(a int, b int);
+insert into t2 (a) values (1), (2), (3), (4);
 set @@global.super_read_only=1;
 include/stop_slave.inc
 change master to master_connect_retry=20;
 include/start_slave.inc
 insert into t1 values(2);
+update t2 natural join t1 set b=3;
 select * from t1;
 a
 1
 2
+select * from t2;
+a	b
+1	3
+2	3
+3	NULL
+4	NULL
 set @@global.read_only_slave=0;
 set @@global.super_read_only=0;
 set @@global.read_only=0;
 set @@global.read_only_slave=1;
-drop table t1;
+drop table t1, t2;
 include/rpl_end.inc

mysql-test/suite/rpl/t/rpl_ignore_super_read_only.test

@@ -1,5 +1,6 @@
 --echo # This unit test is only to test super_read_only=1 doesn't cause
---echo # errors when executing "CHANGE MASTER".
+--echo # errors when executing "CHANGE MASTER" or from statements executed
+--echo # via replication.
 --echo #
 --echo # It is not intended to test general super_read_only functionality.
 --echo #
@@ -11,6 +12,8 @@ source include/master-slave.inc;
 connection master;
 create table t1(a int);
 insert into t1 values(1);
+create table t2(a int, b int);
+insert into t2 (a) values (1), (2), (3), (4);
 sync_slave_with_master;
 
 set @@global.super_read_only=1;
@@ -20,15 +23,17 @@ source include/start_slave.inc;
 
 connection master;
 insert into t1 values(2);
+update t2 natural join t1 set b=3;
 sync_slave_with_master;
 
 select * from t1;
+select * from t2;
 set @@global.read_only_slave=0;
 set @@global.super_read_only=0;
 set @@global.read_only=0;
 set @@global.read_only_slave=1;
 
 connection master;
-drop table t1;
+drop table t1, t2;
 
 source include/rpl_end.inc;

sql/handler.cc

@@ -40,6 +40,7 @@
 #include "debug_sync.h"         // DEBUG_SYNC
 #include <my_bit.h>
 #include <list>
+#include "sql_readonly.h"       // check_ro
 
 #ifdef WITH_PARTITION_STORAGE_ENGINE
 #include "ha_partition.h"
@@ -1469,18 +1470,10 @@ int ha_commit_trans(THD *thd, bool all, bool async,
       DEBUG_SYNC(thd, "ha_commit_trans_after_acquire_commit_lock");
     }
 
-    bool enforce_ro = true;
-    if (!opt_super_readonly)
-      enforce_ro = !(thd->security_ctx->master_access & SUPER_ACL);
     // Ignore super_read_only when ignore_global_read_lock is set.
     // ignore_global_read_lock is set for transactions on replication
     // repository tables.
-    if (ignore_global_read_lock)
-      enforce_ro = false;
-    if (rw_trans &&
-        opt_readonly &&
-        enforce_ro &&
-        !thd->slave_thread)
+    if (rw_trans && check_ro(thd) && !ignore_global_read_lock)
     {
       if (opt_super_readonly)
       {

sql/lock.cc

@@ -83,6 +83,8 @@
 #include <hash.h>
 #include <assert.h>
 #include "my_atomic.h"
+#include "sql_readonly.h"                  // check_ro
+
 /**
   @defgroup Locking Locking
   @{
@@ -116,7 +118,6 @@ static int
 lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags)
 {
   uint system_count= 0, i= 0;
-  bool enforce_ro = true;
   /*
     Identifies if the executed sql command can updated either a log
     or rpl info table.
@@ -125,8 +126,6 @@ lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags)
 
   DBUG_ENTER("lock_tables_check");
 
-  if (!opt_super_readonly)
-    enforce_ro = !(thd->security_ctx->master_access & SUPER_ACL);
   log_table_write_query=
      is_log_table_write_query(thd->lex->sql_command);
 
@@ -197,10 +196,10 @@ lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags)
     */
     if (!(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) && !t->s->tmp_table)
     {
-      if (t->reginfo.lock_type >= TL_WRITE_ALLOW_WRITE &&
-          enforce_ro && opt_readonly && !thd->slave_thread)
+      if (t->reginfo.lock_type >= TL_WRITE_ALLOW_WRITE && check_ro(thd))
       {
-        my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
+        my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
+                 opt_super_readonly ? "--read-only (super)" : "--read-only");
         DBUG_RETURN(1);
       }
     }

sql/sql_parse.cc

@@ -104,6 +104,7 @@
 #include "sql_timer.h"   // thd_timer_set, thd_timer_reset
 #include "sp_rcontext.h"
 
+#include "sql_readonly.h" // check_ro
 
 #include "sql_digest.h"
 
@@ -3857,12 +3858,7 @@ case SQLCOM_PREPARE:
 #endif /* HAVE_REPLICATION */
       if (res)
         break;
-      bool enforce_ro = true;
-      if (!opt_super_readonly)
-        enforce_ro = !(thd->security_ctx->master_access & SUPER_ACL);
-      if (opt_readonly &&
-	  enforce_ro &&
-	  some_non_temp_table_to_be_updated(thd, all_tables))
+      if (check_ro(thd) && some_non_temp_table_to_be_updated(thd, all_tables))
       {
         if (opt_super_readonly)
         {

sql/sql_readonly.h

@@ -0,0 +1,18 @@
+#ifndef SQL_READONLY_INCLUDED
+#define SQL_READONLY_INCLUDED
+
+#include "sql_acl.h"   /* SUPER_ACL */
+#include "sql_class.h" /* THD class */
+#include "mysqld.h"    /* opt_readonly and opt_super_readonly */
+
+static inline bool check_ro(THD *thd)
+{
+  return (
+    opt_readonly && (
+      !(thd->security_ctx->master_access & SUPER_ACL) ||
+      (opt_super_readonly && !(thd->slave_thread))
+    )
+  );
+}
+
+#endif /* SQL_READONLY_INCLUDED */

sql/sql_trigger.cc

@@ -33,6 +33,7 @@
 #include "sql_handler.h"                        // mysql_ha_rm_tables
 #include "sp_cache.h"                     // sp_invalidate_cache
 #include <mysys_err.h>
+#include "sql_readonly.h"                 // check_ro
 
 /*************************************************************************/
 
@@ -444,9 +445,6 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
   if (!create)
   {
     bool if_exists= thd->lex->drop_if_exists;
-    bool enforce_ro= true;
-    if (!opt_super_readonly)
-      enforce_ro= !(thd->security_ctx->master_access & SUPER_ACL);
 
     /*
       Protect the query table list from the temporary and potentially
@@ -461,7 +459,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
     */
     thd->lex->sql_command= backup.sql_command;
 
-    if (opt_readonly && enforce_ro && !thd->slave_thread)
+    if (check_ro(thd))
     {
         my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
                  opt_super_readonly ? "--read-only (super)" : "--read-only");

sql/transaction.cc

@@ -20,6 +20,7 @@
 #include "rpl_master.h"
 #include "debug_sync.h"         // DEBUG_SYNC
 #include "sql_acl.h"            // SUPER_ACL
+#include "sql_readonly.h"       // check_ro
 
 /**
   Check if we have a condition where the transaction state must
@@ -172,10 +173,7 @@ bool trans_begin(THD *thd, uint flags, bool* need_ok)
       Implicitly starting a RW transaction is allowed for backward
       compatibility.
     */
-    bool enforce_ro = true;
-    if (!opt_super_readonly)
-      enforce_ro = !(thd->security_ctx->master_access & SUPER_ACL);
-    if (opt_readonly && enforce_ro)
+    if (check_ro(thd))
     {
       my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
                opt_super_readonly ? "--read-only (super)" : "--read-only");