WL#2971 "change log-bin-trust-routine-creators=0 to apply only to functions".

Indeed now that stored procedures CALL is not binlogged, but instead the invoked substatements are,
the restrictions applied by log-bin-trust-routine-creators=0 are superfluous for procedures.
They still need to apply to functions where function calls are written to the binlog (for example as "DO myfunc(3)").
We rename the variable to log-bin-trust-function-creators but allow the old name until some future version (and issue a warning if old name is used).


mysql-test/mysql-test-run.pl:
  update to new option name
mysql-test/mysql-test-run.sh:
  update to new option name
mysql-test/mysql_test_run_new.c:
  update to new option name
mysql-test/r/rpl_sp.result:
  result update
mysql-test/t/rpl_sp-slave.opt:
  we need to skip this error to not hit BUG#14769
mysql-test/t/rpl_sp.test:
  Test update:
  1) as log-bin-trust-routine-creators now affects only functions, the testing of this option, which was
  mainly done on procedures, is moved to functions
  2) cleanup is simplified; and instead of many SHOW BINLOG EVENTS we do a big one in the end, which is more
  maintainable.
  3) we test a few more function and procedures cases to see how they replicate.
  4) removing out-of-date comments
sql/item_func.cc:
  This warning is wrong since binlogging of functions was changed in August. If a function fails
  in the middle, it will be binlogged with its error code (i.e. properly).
sql/mysql_priv.h:
  variable name changed
sql/mysqld.cc:
  option name changes. A precision about --read-only.
sql/set_var.cc:
  a new class sys_var_trust_routine_creators to be able to issue a "this is a deprecated variable" warning if used.
sql/set_var.h:
  new class to be able to issue a "this is a deprecated variable" warning if used.
sql/share/errmsg.txt:
  routine -> function
sql/sp.cc:
  log-bin-trust-routine-creators now applies only to functions.
sql/sql_parse.cc:
  1) sending ER_FAILED_ROUTINE_BREAK_BINLOG is wrong since August as we don't binlog CALL anymore but instead binlog the substatements;
  the clear_error() goes away too as it was necessary only when we created a binlog event from the "CALL" statement.
  2) log-bin-trust-routine-creators now applies only to functions.
sql/sql_trigger.cc:
  comments.

9 files changed, 77 insertions, 38 deletions

diff --git a/sql/item_func.cc b/sql/item_func.cc
index f467981540b..9201eb9dda2 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -4775,12 +4775,6 @@ Item_func_sp::execute(Item **itp)
   res= m_sp->execute_function(thd, args, arg_count, itp);
   thd->restore_sub_statement_state(&statement_state);
 
-  if (res && mysql_bin_log.is_open() &&
-      (m_sp->m_chistics->daccess == SP_CONTAINS_SQL ||
-       m_sp->m_chistics->daccess == SP_MODIFIES_SQL_DATA))
-    push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
-                 ER_FAILED_ROUTINE_BREAK_BINLOG,
-		 ER(ER_FAILED_ROUTINE_BREAK_BINLOG));
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   sp_restore_security_context(thd, save_ctx_func);
 error:
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index b7bd34c613f..64b0d820a7f 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -1180,7 +1180,7 @@ extern my_bool opt_enable_named_pipe, opt_sync_frm, opt_allow_suspicious_udfs;
 extern my_bool opt_secure_auth;
 extern my_bool opt_log_slow_admin_statements;
 extern my_bool sp_automatic_privileges, opt_noacl;
-extern my_bool opt_old_style_user_limits, trust_routine_creators;
+extern my_bool opt_old_style_user_limits, trust_function_creators;
 extern uint opt_crash_binlog_innodb;
 extern char *shared_memory_base_name, *mysqld_unix_port;
 extern my_bool opt_enable_shared_memory;
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index acad378353b..dc7d11ad728 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -371,7 +371,7 @@ my_bool opt_log_slow_admin_statements= 0;
 my_bool lower_case_file_system= 0;
 my_bool opt_large_pages= 0;
 uint    opt_large_page_size= 0;
-my_bool opt_old_style_user_limits= 0, trust_routine_creators= 0;
+my_bool opt_old_style_user_limits= 0, trust_function_creators= 0;
 /*
   True if there is at least one per-hour limit for some user, so we should
   check them before each query (and possibly reset counters when hour is
@@ -4432,7 +4432,7 @@ enum options_mysqld
   OPT_INNODB_FAST_SHUTDOWN,
   OPT_INNODB_FILE_PER_TABLE, OPT_CRASH_BINLOG_INNODB,
   OPT_INNODB_LOCKS_UNSAFE_FOR_BINLOG,
-  OPT_LOG_BIN_TRUST_ROUTINE_CREATORS,
+  OPT_LOG_BIN_TRUST_FUNCTION_CREATORS,
   OPT_SAFE_SHOW_DB, OPT_INNODB_SAFE_BINLOG,
   OPT_INNODB, OPT_ISAM,
   OPT_ENGINE_CONDITION_PUSHDOWN,
@@ -4857,16 +4857,27 @@ Disable with --skip-innodb-doublewrite.", (gptr*) &innobase_use_doublewrite,
    "File that holds the names for last binary log files.",
    (gptr*) &opt_binlog_index_name, (gptr*) &opt_binlog_index_name, 0, GET_STR,
    REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
+#ifndef TO_BE_REMOVED_IN_5_1_OR_6_0
+  /*
+    In 5.0.6 we introduced the below option, then in 5.0.16 we renamed it to
+    log-bin-trust-function-creators but kept also the old name for
+    compatibility; the behaviour was also changed to apply only to functions
+    (and triggers). In a future release this old name could be removed.
+  */
+  {"log-bin-trust-routine-creators", OPT_LOG_BIN_TRUST_FUNCTION_CREATORS,
+   "(deprecated) Use log-bin-trust-function-creators.",
+   (gptr*) &trust_function_creators, (gptr*) &trust_function_creators, 0,
+   GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
+#endif
   /*
     This option starts with "log-bin" to emphasize that it is specific of
-    binary logging. Hopefully in 5.1 nobody will need it anymore, when we have
-    row-level binlog.
+    binary logging.
   */
-  {"log-bin-trust-routine-creators", OPT_LOG_BIN_TRUST_ROUTINE_CREATORS,
+  {"log-bin-trust-function-creators", OPT_LOG_BIN_TRUST_FUNCTION_CREATORS,
    "If equal to 0 (the default), then when --log-bin is used, creation of "
-   "a routine is allowed only to users having the SUPER privilege and only"
-   "if this routine may not break binary logging",
-   (gptr*) &trust_routine_creators, (gptr*) &trust_routine_creators, 0,
+   "a function is allowed only to users having the SUPER privilege and only "
+   "if this function may not break binary logging.",
+   (gptr*) &trust_function_creators, (gptr*) &trust_function_creators, 0,
    GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
   {"log-error", OPT_ERROR_LOG_FILE, "Error log file.",
    (gptr*) &log_error_file_ptr, (gptr*) &log_error_file_ptr, 0, GET_STR,
@@ -5725,7 +5736,7 @@ The minimum value for this variable is 4096.",
    (gptr*) &max_system_variables.read_buff_size,0, GET_ULONG, REQUIRED_ARG,
    128*1024L, IO_SIZE*2+MALLOC_OVERHEAD, ~0L, MALLOC_OVERHEAD, IO_SIZE, 0},
   {"read_only", OPT_READONLY,
-   "Make all tables readonly, with the exception for replication (slave) threads and users with the SUPER privilege",
+   "Make all non-temporary tables read-only, with the exception for replication (slave) threads and users with the SUPER privilege",
    (gptr*) &opt_readonly,
    (gptr*) &opt_readonly,
    0, GET_BOOL, NO_ARG, 0, 0, 1, 0, 1, 0},
diff --git a/sql/set_var.cc b/sql/set_var.cc
index 8cf7311265c..5a6ff7d05ad 100644
--- a/sql/set_var.cc
+++ b/sql/set_var.cc
@@ -200,9 +200,12 @@ sys_var_key_cache_long  sys_key_cache_age_threshold("key_cache_age_threshold",
 							      param_age_threshold));
 sys_var_bool_ptr	sys_local_infile("local_infile",
 					 &opt_local_infile);
-sys_var_bool_ptr       
+sys_var_trust_routine_creators
 sys_trust_routine_creators("log_bin_trust_routine_creators",
-                           &trust_routine_creators);
+                           &trust_function_creators);
+sys_var_bool_ptr       
+sys_trust_function_creators("log_bin_trust_function_creators",
+                            &trust_function_creators);
 sys_var_thd_ulong	sys_log_warnings("log_warnings", &SV::log_warnings);
 sys_var_thd_ulong	sys_long_query_time("long_query_time",
 					     &SV::long_query_time);
@@ -722,6 +725,7 @@ sys_var *sys_variables[]=
   &sys_innodb_commit_concurrency,
 #endif  
   &sys_trust_routine_creators,
+  &sys_trust_function_creators,
   &sys_engine_condition_pushdown,
 #ifdef HAVE_NDBCLUSTER_DB
   &sys_ndb_autoincrement_prefetch_sz,
@@ -865,7 +869,7 @@ struct show_var_st init_vars[]= {
 #endif
   {"log",                     (char*) &opt_log,                     SHOW_BOOL},
   {"log_bin",                 (char*) &opt_bin_log,                 SHOW_BOOL},
-  {sys_trust_routine_creators.name,(char*) &sys_trust_routine_creators, SHOW_SYS},
+  {sys_trust_function_creators.name,(char*) &sys_trust_function_creators, SHOW_SYS},
   {"log_error",               (char*) log_error_file,               SHOW_CHAR},
 #ifdef HAVE_REPLICATION
   {"log_slave_updates",       (char*) &opt_log_slave_updates,       SHOW_MY_BOOL},
@@ -3463,6 +3467,26 @@ bool process_key_caches(int (* func) (const char *name, KEY_CACHE *))
 }
 
 
+void sys_var_trust_routine_creators::warn_deprecated(THD *thd)
+{
+  push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+		      ER_WARN_DEPRECATED_SYNTAX,
+		      ER(ER_WARN_DEPRECATED_SYNTAX), "log_bin_trust_routine_creators",
+                      "log_bin_trust_function_creators"); 
+}
+
+void sys_var_trust_routine_creators::set_default(THD *thd, enum_var_type type)
+{
+  warn_deprecated(thd);
+  sys_var_bool_ptr::set_default(thd, type);
+}
+
+bool sys_var_trust_routine_creators::update(THD *thd, set_var *var)
+{
+  warn_deprecated(thd);
+  return sys_var_bool_ptr::update(thd, var);
+}
+
 /****************************************************************************
   Used templates
 ****************************************************************************/
diff --git a/sql/set_var.h b/sql/set_var.h
index 854409c159e..18c3353e8ff 100644
--- a/sql/set_var.h
+++ b/sql/set_var.h
@@ -738,6 +738,17 @@ public:
   byte *value_ptr(THD *thd, enum_var_type type, LEX_STRING *base);
 };
 
+class sys_var_trust_routine_creators :public sys_var_bool_ptr
+{
+  /* We need a derived class only to have a warn_deprecated() */
+public:
+  sys_var_trust_routine_creators(const char *name_arg, my_bool *value_arg) :
+    sys_var_bool_ptr(name_arg, value_arg) {};
+  void warn_deprecated(THD *thd);
+  void set_default(THD *thd, enum_var_type type);
+  bool update(THD *thd, set_var *var);
+};
+
 /****************************************************************************
   Classes for parsing of the SET command
 ****************************************************************************/
diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt
index f85bda90e81..8497e236235 100644
--- a/sql/share/errmsg.txt
+++ b/sql/share/errmsg.txt
@@ -5349,9 +5349,9 @@ ER_CANT_CREATE_GEOMETRY_OBJECT 22003
 ER_FAILED_ROUTINE_BREAK_BINLOG
 	eng "A routine failed and has neither NO SQL nor READS SQL DATA in its declaration and binary logging is enabled; if non-transactional tables were updated, the binary log will miss their changes"
 ER_BINLOG_UNSAFE_ROUTINE
-	eng "This routine has none of DETERMINISTIC, NO SQL, or READS SQL DATA in its declaration and binary logging is enabled (you *might* want to use the less safe log_bin_trust_routine_creators variable)"
+	eng "This function has none of DETERMINISTIC, NO SQL, or READS SQL DATA in its declaration and binary logging is enabled (you *might* want to use the less safe log_bin_trust_function_creators variable)"
 ER_BINLOG_CREATE_ROUTINE_NEED_SUPER
-	eng "You do not have the SUPER privilege and binary logging is enabled (you *might* want to use the less safe log_bin_trust_routine_creators variable)"
+	eng "You do not have the SUPER privilege and binary logging is enabled (you *might* want to use the less safe log_bin_trust_function_creators variable)"
 ER_EXEC_STMT_WITH_OPEN_CURSOR
 	eng "You can't execute a prepared statement which has an open cursor associated with it. Reset the statement to re-execute it."
 ER_STMT_HAS_NO_OPEN_CURSOR
diff --git a/sql/sp.cc b/sql/sp.cc
index 8386c5d58a2..e01d05cf405 100644
--- a/sql/sp.cc
+++ b/sql/sp.cc
@@ -551,12 +551,13 @@ db_create_routine(THD *thd, int type, sp_head *sp)
 	store(sp->m_chistics->comment.str, sp->m_chistics->comment.length,
 	      system_charset_info);
 
-    if (!trust_routine_creators && mysql_bin_log.is_open())
+    if ((sp->m_type == TYPE_ENUM_FUNCTION) &&
+        !trust_function_creators && mysql_bin_log.is_open())
     {
       if (!sp->m_chistics->detistic)
       {
 	/*
-	  Note that for a _function_ this test is not enough; one could use
+	  Note that this test is not perfect; one could use
 	  a non-deterministic read-only function in an update statement.
 	*/
 	enum enum_sp_data_access access=
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 4bbca55f6ba..6677b8a09ad 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -4318,18 +4318,6 @@ end_with_restore_list:
           So just execute the statement.
         */
 	res= sp->execute_procedure(thd, &lex->value_list);
-        if (mysql_bin_log.is_open() &&
-            (sp->m_chistics->daccess == SP_CONTAINS_SQL ||
-             sp->m_chistics->daccess == SP_MODIFIES_SQL_DATA))
-        {
-          if (res)
-            push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
-                         ER_FAILED_ROUTINE_BREAK_BINLOG,
-			 ER(ER_FAILED_ROUTINE_BREAK_BINLOG));
-          else
-            thd->clear_error();
-        }
-
 	/*
           If warnings have been cleared, we have to clear total_warn_count
           too, otherwise the clients get confused.
@@ -4388,7 +4376,8 @@ end_with_restore_list:
         if (end_active_trans(thd)) 
           goto error;
 	memcpy(&lex->sp_chistics, &chistics, sizeof(lex->sp_chistics));
-        if (!trust_routine_creators &&  mysql_bin_log.is_open() &&
+        if ((sp->m_type == TYPE_ENUM_FUNCTION) &&
+            !trust_function_creators &&  mysql_bin_log.is_open() &&
             !sp->m_chistics->detistic &&
             (chistics.daccess == SP_CONTAINS_SQL ||
              chistics.daccess == SP_MODIFIES_SQL_DATA))
@@ -4399,6 +4388,12 @@ end_with_restore_list:
         }
         else
         {
+          /*
+            Note that if you implement the capability of ALTER FUNCTION to
+            alter the body of the function, this command should be made to
+            follow the restrictions that log-bin-trust-function-creators=0
+            already puts on CREATE FUNCTION.
+          */
           if (lex->sql_command == SQLCOM_ALTER_PROCEDURE)
             result= sp_update_procedure(thd, lex->spname, &lex->sp_chistics);
           else
diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc
index dbad8dcffb5..3eaa067807a 100644
--- a/sql/sql_trigger.cc
+++ b/sql/sql_trigger.cc
@@ -131,9 +131,12 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
     But a trigger can in theory be used to do nasty things (if it supported
     DROP for example) so we do the check for privileges. For now there is
     already a stronger test right above; but when this stronger test will
-    be removed, the test below will hold.
+    be removed, the test below will hold. Because triggers have the same
+    nature as functions regarding binlogging: their body is implicitely
+    binlogged, so they share the same danger, so trust_function_creators
+    applies to them too.
   */
-  if (!trust_routine_creators && mysql_bin_log.is_open() &&
+  if (!trust_function_creators && mysql_bin_log.is_open() &&
       !(thd->security_ctx->master_access & SUPER_ACL))
   {
     my_error(ER_BINLOG_CREATE_ROUTINE_NEED_SUPER, MYF(0));