Allow mysql_upgrade to enable event after table is corrected

new features:
set event_scheduler=ON|OFF will now try to init event scheduler
if it's not enabled
set event_scheduler=default will try to enable it based on
the value of the event_scheduler when mysqld was started

client/mysql_upgrade.c

@@ -865,6 +865,7 @@ static const char *expected_errors[]=
   "ERROR 1060", /* Duplicate column name */
   "ERROR 1061", /* Duplicate key name */
   "ERROR 1054", /* Unknown column */
+  "ERROR 1290", /* RR_OPTION_PREVENTS_STATEMENT */
   0
 };
 

mysql-test/r/events_restart.result

@@ -18,7 +18,7 @@ change column body  body longtext character set utf8 collate utf8_bin;
 use events_test;
 select @@event_scheduler;
 @@event_scheduler
-DISABLED
+OFF
 show events;
 ERROR HY000: Cannot proceed because system tables used by Event Scheduler were found damaged at server start
 select event_name from information_schema.events;
@@ -40,12 +40,12 @@ ERROR HY000: Cannot proceed because system tables used by Event Scheduler were f
 drop event intact_check;
 ERROR HY000: Cannot proceed because system tables used by Event Scheduler were found damaged at server start
 set global event_scheduler=on;
-ERROR HY000: Cannot proceed because system tables used by Event Scheduler were found damaged at server start
+ERROR HY000: Event Scheduler: An error occurred when initializing system tables. Disabling the Event Scheduler.
 set global event_scheduler=off;
-ERROR HY000: Cannot proceed because system tables used by Event Scheduler were found damaged at server start
+ERROR HY000: Event Scheduler: An error occurred when initializing system tables. Disabling the Event Scheduler.
 show variables like 'event_scheduler';
 Variable_name	Value
-event_scheduler	DISABLED
+event_scheduler	OFF
 Make sure that we still can create and drop databases,
 and no warnings are produced.
 drop database if exists mysqltest_database_not_exists;
@@ -58,6 +58,22 @@ Error	1545	Failed to open mysql.event
 Restore the original mysql.event table
 drop table mysql.event;
 rename table event_like to mysql.event;
+check that we can now enable events without restart
+set global event_scheduler=original;
+Warnings:
+Note	1408	Event Scheduler: Loaded 3 events
+select @@global.event_scheduler;
+@@global.event_scheduler
+ON
+set global event_scheduler=on;
+select @@global.event_scheduler;
+@@global.event_scheduler
+ON
+show events;
+Db	Name	Definer	Time zone	Type	Execute at	Interval value	Interval field	Starts	Ends	Status	Originator	character_set_client	collation_connection	Database Collation
+events_test	abc1	root@localhost	SYSTEM	RECURRING	#	1	SECOND	#	#	ENABLED	1	latin1	latin1_swedish_ci	latin1_swedish_ci
+events_test	abc2	root@localhost	SYSTEM	RECURRING	#	1	SECOND	#	#	ENABLED	1	latin1	latin1_swedish_ci	latin1_swedish_ci
+events_test	abc3	root@localhost	SYSTEM	RECURRING	#	1	SECOND	#	#	ENABLED	1	latin1	latin1_swedish_ci	latin1_swedish_ci
 Now let's restart the server again
 use events_test;
 select @@event_scheduler;

mysql-test/r/mysql_upgrade-6984.result

@@ -57,3 +57,4 @@ Phase 6/6: Running 'FLUSH PRIVILEGES'
 OK
 update mysql.user set password='' where user='root';
 flush privileges;
+set global event_scheduler=OFF;

mysql-test/r/skip_grants.result

@@ -59,7 +59,9 @@ DROP FUNCTION f1;
 DROP FUNCTION f2;
 DROP FUNCTION f3;
 set global event_scheduler=1;
-ERROR HY000: The MariaDB server is running with the --event-scheduler=DISABLED or --skip-grant-tables option so it cannot execute this statement
+Warnings:
+Note	1408	Event Scheduler: Loaded 0 events
+set global event_scheduler=0;
 select count(*) from information_schema.COLUMN_PRIVILEGES;
 count(*)
 0

mysql-test/t/events_restart.test

@@ -71,9 +71,9 @@ drop event intact_check_1;
 drop event intact_check_2;
 --error ER_EVENTS_DB_ERROR
 drop event intact_check;
---error ER_EVENTS_DB_ERROR
+--error ER_STARTUP
 set global event_scheduler=on;
---error ER_EVENTS_DB_ERROR
+--error ER_STARTUP
 set global event_scheduler=off;
 show variables like 'event_scheduler';
 --echo Make sure that we still can create and drop databases,
@@ -84,6 +84,16 @@ drop database mysqltest_db1;
 --echo Restore the original mysql.event table
 drop table mysql.event;
 rename table event_like to mysql.event;
+
+--echo check that we can now enable events without restart
+set global event_scheduler=original;
+select @@global.event_scheduler;
+set global event_scheduler=on;
+select @@global.event_scheduler;
+--sorted_result
+--replace_column 6 # 9 # 10 #
+show events;
+
 --echo Now let's restart the server again
 
 --source include/restart_mysqld.inc

mysql-test/t/mysql_upgrade-6984.test

@@ -19,4 +19,5 @@ connect(con1,localhost,root,foo,,,);
 
 update mysql.user set password='' where user='root';
 flush privileges;
-
+# Load event table
+set global event_scheduler=OFF;

mysql-test/t/skip_grants.test

@@ -112,8 +112,8 @@ DROP FUNCTION f3;
 #
 # Bug #26807 "set global event_scheduler=1" and --skip-grant-tables crashes server
 #
---error ER_OPTION_PREVENTS_STATEMENT
 set global event_scheduler=1;
+set global event_scheduler=0;
 
 #
 # Bug#26285 Selecting information_schema crahes server

scripts/mysql_system_tables_fix.sql

@@ -597,6 +597,8 @@ ALTER TABLE event ADD body_utf8 longblob DEFAULT NULL
                       AFTER db_collation;
 ALTER TABLE event MODIFY body_utf8 longblob DEFAULT NULL;
 
+# Enable event scheduler if the event table was not up to date before.
+set global event_scheduler=original;
 
 #
 # TRIGGER privilege

sql/events.cc

@@ -80,7 +80,8 @@ Event_queue *Events::event_queue;
 Event_scheduler *Events::scheduler;
 Event_db_repository *Events::db_repository;
 ulong Events::opt_event_scheduler= Events::EVENTS_OFF;
-bool Events::check_system_tables_error= FALSE;
+ulong Events::startup_state= Events::EVENTS_OFF;
+ulong Events::inited;
 
 
 /*
@@ -114,7 +115,7 @@ bool Events::check_if_system_tables_error()
 {
   DBUG_ENTER("Events::check_if_system_tables_error");
 
-  if (check_system_tables_error)
+  if (!inited)
   {
     my_error(ER_EVENTS_DB_ERROR, MYF(0));
     DBUG_RETURN(TRUE);
@@ -257,10 +258,10 @@ common_1_lev_code:
 
 
 /**
-  Create a new query string for removing executable comments 
+  Create a new query string for removing executable comments
-  for avoiding leak and keeping consistency of the execution 
+  for avoiding leak and keeping consistency of the execution
   on master and slave.
-  
+
   @param[in] thd                 Thread handler
   @param[in] buf                 Query string
 
@@ -281,7 +282,7 @@ create_query_string(THD *thd, String *buf)
                   thd->lex->stmt_definition_end -
                   thd->lex->stmt_definition_begin))
     return 1;
- 
+
   return 0;
 }
 
@@ -336,8 +337,8 @@ Events::create_event(THD *thd, Event_parse_data *parse_data,
 
   if (parse_data->do_not_create)
     DBUG_RETURN(FALSE);
-  /* 
+  /*
-    Turn off row binlogging of this statement and use statement-based 
+    Turn off row binlogging of this statement and use statement-based
     so that all supporting tables are updated for CREATE EVENT command.
   */
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
@@ -384,8 +385,10 @@ Events::create_event(THD *thd, Event_parse_data *parse_data,
       String log_query;
       if (create_query_string(thd, &log_query))
       {
-        sql_print_error("Event Error: An error occurred while creating query "
+        my_message_sql(ER_STARTUP,
-                        "string, before writing it into binary log.");
+                       "Event Error: An error occurred while creating query "
+                       "string, before writing it into binary log.",
+                       MYF(ME_NOREFRESH));
         ret= true;
       }
       else
@@ -473,8 +476,8 @@ Events::update_event(THD *thd, Event_parse_data *parse_data,
     }
   }
 
-  /* 
+  /*
-    Turn off row binlogging of this statement and use statement-based 
+    Turn off row binlogging of this statement and use statement-based
     so that all supporting tables are updated for UPDATE EVENT command.
   */
   save_binlog_format= thd->set_current_stmt_binlog_format_stmt();
@@ -752,6 +755,13 @@ Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
   int ret;
   DBUG_ENTER("Events::fill_schema_events");
 
+  /*
+    If we didn't start events because of --skip-grant-tables, return an
+    empty set
+  */
+  if (opt_noacl)
+    DBUG_RETURN(0);
+
   if (check_if_system_tables_error())
     DBUG_RETURN(1);
 
@@ -780,6 +790,7 @@ Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
 /**
   Initializes the scheduler's structures.
 
+  @param  THD or null (if called by init)
   @param  opt_noacl_or_bootstrap
                      TRUE if there is --skip-grant-tables or --bootstrap
                      option. In that case we disable the event scheduler.
@@ -787,44 +798,56 @@ Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
   @note   This function is not synchronized.
 
   @retval  FALSE   Perhaps there was an error, and the event scheduler
-                   is disabled. But the error is not fatal and the 
+                   is disabled. But the error is not fatal and the
                    server start up can continue.
   @retval  TRUE    Fatal error. Startup must terminate (call unireg_abort()).
 */
 
 bool
-Events::init(bool opt_noacl_or_bootstrap)
+Events::init(THD *thd, bool opt_noacl_or_bootstrap)
 {
-
-  THD *thd;
   int err_no;
   bool res= FALSE;
-
+  bool had_thd= thd != 0;
   DBUG_ENTER("Events::init");
 
+  DBUG_ASSERT(inited == 0);
+
+  /*
+    Was disabled explicitly from the command line
+  */
+  if (opt_event_scheduler == Events::EVENTS_DISABLED ||
+      opt_noacl_or_bootstrap)
+    DBUG_RETURN(FALSE);
+
   /* We need a temporary THD during boot */
-  if (!(thd= new THD()))
+  if (!thd)
   {
-    res= TRUE;
+
-    goto end;
+    if (!(thd= new THD()))
+    {
+      res= TRUE;
+      goto end;
+    }
+    /*
+      The thread stack does not start from this function but we cannot
+      guess the real value. So better some value that doesn't assert than
+      no value.
+    */
+    thd->thread_stack= (char*) &thd;
+    thd->store_globals();
+    /*
+      Set current time for the thread that handles events.
+      Current time is stored in data member start_time of THD class.
+      Subsequently, this value is used to check whether event was expired
+      when make loading events from storage. Check for event expiration time
+      is done at Event_queue_element::compute_next_execution_time() where
+      event's status set to Event_parse_data::DISABLED and dropped flag set
+      to true if event was expired.
+    */
+    thd->set_time();
   }
-  /*
+
-    The thread stack does not start from this function but we cannot
-    guess the real value. So better some value that doesn't assert than
-    no value.
-  */
-  thd->thread_stack= (char*) &thd;
-  thd->store_globals();
-  /*
-    Set current time for the thread that handles events.
-    Current time is stored in data member start_time of THD class.
-    Subsequently, this value is used to check whether event was expired
-    when make loading events from storage. Check for event expiration time
-    is done at Event_queue_element::compute_next_execution_time() where
-    event's status set to Event_parse_data::DISABLED and dropped flag set
-    to true if event was expired.
-  */
-  thd->set_time();
   /*
     We will need Event_db_repository anyway, even if the scheduler is
     disabled - to perform events DDL.
@@ -844,28 +867,19 @@ Events::init(bool opt_noacl_or_bootstrap)
     are most likely not there and we're going to disable the event
     scheduler anyway.
   */
-  if (opt_noacl_or_bootstrap || Event_db_repository::check_system_tables(thd))
+  if (Event_db_repository::check_system_tables(thd))
   {
-    if (! opt_noacl_or_bootstrap)
+    delete db_repository;
-    {
+    db_repository= 0;
-      sql_print_error("Event Scheduler: An error occurred when initializing "
+    my_message(ER_STARTUP,
-                      "system tables. Disabling the Event Scheduler.");
+               "Event Scheduler: An error occurred when initializing "
-      check_system_tables_error= TRUE;
+               "system tables. Disabling the Event Scheduler.",
-    }
+               MYF(ME_NOREFRESH));
-
     /* Disable the scheduler since the system tables are not up to date */
-    opt_event_scheduler= EVENTS_DISABLED;
+    opt_event_scheduler= EVENTS_OFF;
     goto end;
   }
 
-  /*
-    Was disabled explicitly from the command line, or because we're running
-    with --skip-grant-tables, or --bootstrap, or because we have no system
-    tables.
-  */
-  if (opt_event_scheduler == Events::EVENTS_DISABLED)
-    goto end;
-
 
   DBUG_ASSERT(opt_event_scheduler == Events::EVENTS_ON ||
               opt_event_scheduler == Events::EVENTS_OFF);
@@ -880,22 +894,23 @@ Events::init(bool opt_noacl_or_bootstrap)
   if (event_queue->init_queue(thd) || load_events_from_db(thd) ||
       (opt_event_scheduler == EVENTS_ON && scheduler->start(&err_no)))
   {
-    sql_print_error("Event Scheduler: Error while loading from disk.");
+    my_message_sql(ER_STARTUP,
+                   "Event Scheduler: Error while loading from mysql.event table.",
+                   MYF(ME_NOREFRESH));
     res= TRUE; /* fatal error: request unireg_abort */
     goto end;
   }
   Event_worker_thread::init(db_repository);
+  inited= 1;
 
 end:
   if (res)
+    deinit();
+  if (!had_thd)
   {
-    delete db_repository;
+    delete thd;
-    delete event_queue;
+    set_current_thd(0);
-    delete scheduler;
   }
-  delete thd;
-  /* Remember that we don't have a THD */
-  set_current_thd(0);
 
   DBUG_RETURN(res);
 }
@@ -915,17 +930,14 @@ Events::deinit()
 {
   DBUG_ENTER("Events::deinit");
 
-  if (opt_event_scheduler != EVENTS_DISABLED)
+  delete scheduler;
-  {
+  scheduler= NULL;                            /* For restart */
-    delete scheduler;
+  delete event_queue;
-    scheduler= NULL;                            /* safety */
+  event_queue= NULL;                          /* For restart */
-    delete event_queue;
-    event_queue= NULL;                          /* safety */
-  }
-
   delete db_repository;
-  db_repository= NULL;                          /* safety */
+  db_repository= NULL;                        /* For restart */
 
+  inited= 0;
   DBUG_VOID_RETURN;
 }
 
@@ -1028,7 +1040,7 @@ Events::dump_internal_status()
     holding LOCK_global_system_variables.
   */
   mysql_mutex_lock(&LOCK_global_system_variables);
-  if (opt_event_scheduler == EVENTS_DISABLED)
+  if (!inited)
     puts("The Event Scheduler is disabled");
   else
   {
@@ -1042,11 +1054,13 @@ Events::dump_internal_status()
 
 bool Events::start(int *err_no)
 {
+  DBUG_ASSERT(inited);
   return scheduler->start(err_no);
 }
 
 bool Events::stop()
 {
+  DBUG_ASSERT(inited);
   return scheduler->stop();
 }
 
@@ -1076,7 +1090,6 @@ Events::load_events_from_db(THD *thd)
   bool ret= TRUE;
   uint count= 0;
   ulong saved_master_access;
-
   DBUG_ENTER("Events::load_events_from_db");
   DBUG_PRINT("enter", ("thd: 0x%lx", (long) thd));
 
@@ -1101,7 +1114,9 @@ Events::load_events_from_db(THD *thd)
 
   if (ret)
   {
-    sql_print_error("Event Scheduler: Failed to open table mysql.event");
+    my_message_sql(ER_STARTUP,
+                   "Event Scheduler: Failed to open table mysql.event",
+                   MYF(ME_NOREFRESH));
     DBUG_RETURN(TRUE);
   }
 
@@ -1123,9 +1138,11 @@ Events::load_events_from_db(THD *thd)
 
     if (et->load_from_row(thd, table))
     {
-      sql_print_error("Event Scheduler: "
+      my_message(ER_STARTUP,
-                      "Error while loading events from mysql.event. "
+                 "Event Scheduler: "
-                      "The table probably contains bad data or is corrupted");
+                 "Error while loading events from mysql.event. "
+                 "The table probably contains bad data or is corrupted",
+                 MYF(ME_NOREFRESH));
       delete et;
       goto end;
     }
@@ -1163,9 +1180,12 @@ Events::load_events_from_db(THD *thd)
       }
     }
   }
-  if (global_system_variables.log_warnings)
+  my_printf_error(ER_STARTUP,
-    sql_print_information("Event Scheduler: Loaded %d event%s",
+                  "Event Scheduler: Loaded %d event%s",
-                          count, (count == 1) ? "" : "s");
+                  MYF(ME_NOREFRESH |
+                      (global_system_variables.log_warnings) ?
+                      ME_JUST_INFO: 0),
+                  count, (count == 1) ? "" : "s");
   ret= FALSE;
 
 end:

sql/events.h

@@ -79,9 +79,11 @@ public:
     and the @@global.event_scheduler SQL variable.
     See sys_var.cc
   */
-  enum enum_opt_event_scheduler { EVENTS_OFF, EVENTS_ON, EVENTS_DISABLED };
+  enum enum_opt_event_scheduler { EVENTS_OFF, EVENTS_ON, EVENTS_DISABLED,
+                                  EVENTS_ORIGINAL };
   /* Protected using LOCK_global_system_variables only. */
-  static ulong opt_event_scheduler;
+  static ulong opt_event_scheduler, startup_state;
+  static ulong inited;
   static bool check_if_system_tables_error();
   static bool start(int *err_no);
   static bool stop();
@@ -91,8 +93,7 @@ public:
   static Event_db_repository *
   get_db_repository() { return db_repository; }
 
-  static bool
+  static bool init(THD *thd, bool opt_noacl);
-  init(bool opt_noacl);
 
   static void
   deinit();
@@ -130,6 +131,11 @@ public:
   static void
   dump_internal_status();
 
+  static void set_original_state(ulong startup_state_org)
+  {
+    startup_state= startup_state_org;
+  }
+
 private:
 
   static bool
@@ -139,8 +145,6 @@ private:
   static Event_queue         *event_queue;
   static Event_scheduler     *scheduler;
   static Event_db_repository *db_repository;
-  /* Set to TRUE if an error at start up */
-  static bool check_system_tables_error;
 
 private:
   /* Prevent use of these */

sql/mysqld.cc

@@ -5507,7 +5507,15 @@ int mysqld_main(int argc, char **argv)
 
   execute_ddl_log_recovery();
 
-  if (Events::init(opt_noacl || opt_bootstrap))
+  /*
+    Change EVENTS_ORIGINAL to EVENTS_OFF (the default value) as there is no
+    point in using ORIGINAL during startup
+  */
+  if (Events::opt_event_scheduler == Events::EVENTS_ORIGINAL)
+    Events::opt_event_scheduler= Events::EVENTS_OFF;
+
+  Events::set_original_state(Events::opt_event_scheduler);
+  if (Events::init((THD*) 0, opt_noacl || opt_bootstrap))
     unireg_abort(1);
 
   if (opt_bootstrap)

sql/sql_error.cc

@@ -510,8 +510,10 @@ Diagnostics_area::set_error_status(uint sql_errno,
 void
 Diagnostics_area::disable_status()
 {
+  DBUG_ENTER("disable_status");
   DBUG_ASSERT(! is_set());
   m_status= DA_DISABLED;
+  DBUG_VOID_RETURN;
 }
 
 Warning_info::Warning_info(ulonglong warn_id_arg,

sql/sys_vars.cc

@@ -815,30 +815,26 @@ static Sys_var_ulong Sys_delayed_queue_size(
        VALID_RANGE(1, UINT_MAX), DEFAULT(DELAYED_QUEUE_SIZE), BLOCK_SIZE(1));
 
 #ifdef HAVE_EVENT_SCHEDULER
-static const char *event_scheduler_names[]= { "OFF", "ON", "DISABLED", NullS };
+static const char *event_scheduler_names[]= { "OFF", "ON", "DISABLED",
+                                              "ORIGINAL", NullS };
 static bool event_scheduler_check(sys_var *self, THD *thd, set_var *var)
 {
-  /* DISABLED is only accepted on the command line */
-  if (var->save_result.ulonglong_value == Events::EVENTS_DISABLED)
-    return true;
-  /*
-    If the scheduler was disabled because there are no/bad
-    system tables, produce a more meaningful error message
-    than ER_OPTION_PREVENTS_STATEMENT
-  */
-  if (Events::check_if_system_tables_error())
-    return true;
   if (Events::opt_event_scheduler == Events::EVENTS_DISABLED)
   {
     my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
              "--event-scheduler=DISABLED or --skip-grant-tables");
     return true;
   }
+  /* DISABLED is only accepted on the command line */
+  if (var->save_result.ulonglong_value == Events::EVENTS_DISABLED)
+    return true;
   return false;
 }
+
 static bool event_scheduler_update(sys_var *self, THD *thd, enum_var_type type)
 {
   int err_no= 0;
+  bool ret;
   uint opt_event_scheduler_value= Events::opt_event_scheduler;
   mysql_mutex_unlock(&LOCK_global_system_variables);
   /*
@@ -857,9 +853,25 @@ static bool event_scheduler_update(sys_var *self, THD *thd, enum_var_type type)
     rare and it's difficult to avoid it without opening up possibilities
     for deadlocks. See bug#51160.
   */
-  bool ret= opt_event_scheduler_value == Events::EVENTS_ON
+
-            ? Events::start(&err_no)
+  /* EVENTS_ORIGINAL means we should revert back to the startup state */
-            : Events::stop();
+  if (opt_event_scheduler_value == Events::EVENTS_ORIGINAL)
+  {
+    opt_event_scheduler_value= Events::opt_event_scheduler=
+      Events::startup_state;
+  }
+ 
+  /*
+    If the scheduler was not properly inited (because of wrong system tables),
+    try to init it again. This is needed for mysql_upgrade to work properly if
+    the event tables where upgraded.
+  */
+  if (!Events::inited && (Events::init(thd, 0) || !Events::inited))
+    ret= 1;
+  else
+    ret= opt_event_scheduler_value == Events::EVENTS_ON ?
+      Events::start(&err_no) :
+      Events::stop();
   mysql_mutex_lock(&LOCK_global_system_variables);
   if (ret)
   {