BUG#53437 @@session.sql_bin_log support in substatements is incorrect

      
The thd->variables.option_bits & OPTION_BIN_LOG is currently abused: 
it's both a system variable and an implementation switch. The current
approach to this option bit breaks the session variable encapsulation. 
      
Besides it is allowed to change @@session.sql_bin_log within a
transaction what may lead to not correctly logging a transaction.
      
To fix the problems,  we created a thd->variables variable to represent
the "sql_log_bin" and prohibited its update inside a transaction or
sub-statement.

mysql-test/suite/binlog/r/binlog_stm_unsafe_warning.result:
  Updated result file. The reason the warnings are removed is
  related to BUG#50312.
mysql-test/suite/binlog/r/binlog_switch_inside_trans.result:
  Checks when is possible to change the option @@session.sql_log_bin.
mysql-test/suite/binlog/t/binlog_switch_inside_trans.test:
  Checks when is possible to change the option @@session.sql_log_bin.
mysql-test/suite/rpl/r/rpl_non_direct_stm_mixing_engines.result:
  Updated the result file with warnings that were being printed
  due to the wrong use of the thd->variables.option_bits and
  sql_log_bin_top_level variables.
mysql-test/suite/rpl/r/rpl_stm_mixing_engines.result:
  Updated the result file with warnings that were being printed
  due to the wrong use of the thd->variables.option_bits and
  sql_log_bin_top_level variables.
sql/share/errmsg-utf8.txt:
  Introduces two error messages to notify that the @@session.sql_log_bin
  cannot be changed inside a sub-statement or transaction.
sql/sql_base.cc:
  Removes the variable sql_log_bin_toplevel and uses
  the session variable sql_log_bin.
sql/sql_class.cc:
  Replaces the variable sql_log_bin_toplevel by the
  (variables.option_bits & OPTION_BIN_LOG).
sql/sql_class.h:
  Removes the variable sql_log_bin_toplevel and creates 
  a session variable sql_log_bin.
sql/sys_vars.cc:
  Checks when the sql_log_bin can be correctly updated.

5 files changed, 82 insertions, 15 deletions

diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
index 598f563ac87..f6ed6330749 100644
--- a/sql/share/errmsg-utf8.txt
+++ b/sql/share/errmsg-utf8.txt
@@ -6336,3 +6336,11 @@ ER_BINLOG_UNSAFE_MULTIPLE_ENGINES_AND_SELF_LOGGING_ENGINE
 
 ER_BINLOG_UNSAFE_MIXED_STATEMENT
   eng "Statements that read from both transactional (or a temporary table of any engine type) and non-transactional tables and write to any of them are unsafe."
+
+ER_INSIDE_TRANSACTION_PREVENTS_SWITCH_SQL_LOG_BIN
+  eng "Cannot modify @@session.sql_log_bin inside a transaction"
+
+ER_STORED_FUNCTION_PREVENTS_SWITCH_SQL_LOG_BIN
+  eng "Cannot change the sql_log_bin inside a stored function or trigger"
+
+
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 28633365e28..dd425aaec1f 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -4001,9 +4001,9 @@ thr_lock_type read_lock_type_for_table(THD *thd,
     prelocked mode we can't rely on OPTION_BIN_LOG flag in THD::options
     bitmap to determine that binary logging is turned on as this bit can
     be cleared before executing sub-statement. So instead we have to look
-    at THD::sql_log_bin_toplevel member.
+    at THD::variables::sql_log_bin member.
   */
-  bool log_on= mysql_bin_log.is_open() && thd->sql_log_bin_toplevel;
+  bool log_on= mysql_bin_log.is_open() && thd->variables.sql_log_bin;
   ulong binlog_format= thd->variables.binlog_format;
   if ((log_on == FALSE) || (binlog_format == BINLOG_FORMAT_ROW) ||
       (table_list->table->s->table_category == TABLE_CATEGORY_LOG) ||
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 789b01443f7..cd70e13f9d4 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -460,7 +460,6 @@ THD::THD()
    rli_fake(0),
    lock_id(&main_lock_id),
    user_time(0), in_sub_stmt(0),
-   sql_log_bin_toplevel(false),
    binlog_unsafe_warning_flags(0), binlog_table_maps(0),
    table_map_for_update(0),
    arg_of_last_insert_id_function(FALSE),
@@ -929,7 +928,11 @@ void THD::init(void)
   update_charset();
   reset_current_stmt_binlog_format_row();
   bzero((char *) &status_var, sizeof(status_var));
-  sql_log_bin_toplevel= variables.option_bits & OPTION_BIN_LOG;
+
+  if (variables.sql_log_bin)
+    variables.option_bits|= OPTION_BIN_LOG;
+  else
+    variables.option_bits&= ~OPTION_BIN_LOG;
 
 #if defined(ENABLED_DEBUG_SYNC)
   /* Initialize the Debug Sync Facility. See debug_sync.cc. */
@@ -4554,8 +4557,13 @@ int THD::binlog_query(THD::enum_binlog_query_type qtype, char const *query_arg,
     because the warnings should be printed only if the statement is
     actually logged. When executing decide_logging_format(), we cannot
     know for sure if the statement will be logged.
+
+    Besides, we should not try to print these warnings if it is not
+    possible to write statements to the binary log as it happens when
+    the execution is inside a function, or generaly speaking, when
+    the variables.option_bits & OPTION_BIN_LOG is false.
   */
-  if (sql_log_bin_toplevel)
+  if (variables.option_bits & OPTION_BIN_LOG)
     issue_unsafe_warnings();
 
   switch (qtype) {
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 916b79f8353..c7990e5d647 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -428,6 +428,7 @@ typedef struct system_variables
 
   uint binlog_format; ///< binlog format for this thd (see enum_binlog_format)
   my_bool binlog_direct_non_trans_update;
+  my_bool sql_log_bin;
   uint completion_type;
   uint query_cache_type;
   uint tx_isolation;
@@ -1667,8 +1668,6 @@ public:
 
   /* <> 0 if we are inside of trigger or stored function. */
   uint in_sub_stmt;
-  /* TRUE when the current top has SQL_LOG_BIN ON */
-  bool sql_log_bin_toplevel;
 
   /* container for handler's private per-connection data */
   Ha_data ha_data[MAX_HA];
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index b8312fc3255..c0938f81b50 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -2236,17 +2236,69 @@ static Sys_var_bit Sys_log_off(
        SESSION_VAR(option_bits), NO_CMD_LINE, OPTION_LOG_OFF,
        DEFAULT(FALSE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_has_super));
 
-static bool fix_sql_log_bin(sys_var *self, THD *thd, enum_var_type type)
+/**
+  This function sets the session variable thd->variables.sql_log_bin 
+  to reflect changes to @@session.sql_log_bin.
+
+  @param[IN] self   A pointer to the sys_var, i.e. Sys_log_binlog.
+  @param[IN] type   The type either session or global.
+
+  @return @c FALSE.
+*/
+static bool fix_sql_log_bin_after_update(sys_var *self, THD *thd,
+                                         enum_var_type type)
 {
-  if (type != OPT_GLOBAL && !thd->in_sub_stmt)
-    thd->sql_log_bin_toplevel= thd->variables.option_bits & OPTION_BIN_LOG;
-  return false;
+  if (type == OPT_SESSION)
+  {
+    if (thd->variables.sql_log_bin)
+      thd->variables.option_bits |= OPTION_BIN_LOG;
+    else
+      thd->variables.option_bits &= ~OPTION_BIN_LOG;
+  }
+  return FALSE;
 }
-static Sys_var_bit Sys_log_binlog(
+
+/**
+  This function checks if the sql_log_bin can be changed,
+  what is possible if:
+    - the user is a super user;
+    - the set is not called from within a function/trigger;
+    - there is no on-going transaction.
+
+  @param[IN] self   A pointer to the sys_var, i.e. Sys_log_binlog.
+  @param[IN] var    A pointer to the set_var created by the parser.
+
+  @return @c FALSE if the change is allowed, otherwise @c TRUE.
+*/
+static bool check_sql_log_bin(sys_var *self, THD *thd, set_var *var)
+{
+  if (check_has_super(self, thd, var))
+    return TRUE;
+
+  if (var->type == OPT_GLOBAL)
+    return FALSE;
+
+  /* If in a stored function/trigger, it's too late to change sql_log_bin. */
+  if (thd->in_sub_stmt)
+  {
+    my_error(ER_STORED_FUNCTION_PREVENTS_SWITCH_SQL_LOG_BIN, MYF(0));
+    return TRUE;
+  }
+  /* Make the session variable 'sql_log_bin' read-only inside a transaction. */
+  if (thd->in_active_multi_stmt_transaction())
+  {
+    my_error(ER_INSIDE_TRANSACTION_PREVENTS_SWITCH_SQL_LOG_BIN, MYF(0));
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+static Sys_var_mybool Sys_log_binlog(
        "sql_log_bin", "sql_log_bin",
-       SESSION_VAR(option_bits), NO_CMD_LINE, OPTION_BIN_LOG,
-       DEFAULT(TRUE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_has_super),
-       ON_UPDATE(fix_sql_log_bin));
+       SESSION_VAR(sql_log_bin), NO_CMD_LINE,
+       DEFAULT(TRUE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_sql_log_bin),
+       ON_UPDATE(fix_sql_log_bin_after_update));
 
 static Sys_var_bit Sys_sql_warnings(
        "sql_warnings", "sql_warnings",