A set of changes aiming to make the Event Scheduler more user-friendly

when there are no up-to-date system tables to support it:
 - initialize the scheduler before reporting "Ready for connections".
   This ensures that warnings, if any, are printed before "Ready for
   connections", and this message is not mangled.
 - do not abort the scheduler if there are no system tables
 - check the tables once at start up, remember the status and disable
   the scheduler if the tables are not up to date.
   If one attempts to use the scheduler with bad tables,
   issue an error message.
 - clean up the behaviour of the module under LOCK TABLES and pre-locking
   mode
 - make sure implicit commit of Events DDL works as expected.
 - add more tests


Collateral clean ups in the events code.

This patch fixes Bug#23631 Events: SHOW VARIABLES doesn't work 
when mysql.event is damaged


mysql-test/r/events.result:
  Update results.
mysql-test/r/events_bugs.result:
  Update results.
mysql-test/r/events_restart_phase1.result:
  Update results.
mysql-test/r/events_restart_phase2.result:
  Update results.
mysql-test/r/events_restart_phase3.result:
  Update results.
mysql-test/r/events_scheduling.result:
  Update results.
mysql-test/r/events_time_zone.result:
  Update results.
mysql-test/t/events.test:
  Add new tests for tampering with mysql.event and some more
  tests for sub-statements, LOCK TABLES mode and pre-locking.
mysql-test/t/events_bugs.test:
  Move the non-concurrent part of test for Bug 16420 to this file.
mysql-test/t/events_restart_phase1.test:
  Rewrite events_restart_* tests to take into account that now
  we check mysql.event table only once, at server startup.
mysql-test/t/events_restart_phase2.test:
  Rewrite events_restart_* tests to take into account that now
  we check mysql.event table only once, at server startup.
mysql-test/t/events_restart_phase3.test:
  Rewrite events_restart_* tests to take into account that now
  we check mysql.event table only once, at server startup.
mysql-test/t/events_scheduling.test:
  Add more coverage for event_scheduler global variable.
mysql-test/t/events_time_zone.test:
  Move the non-concurrent part of the tests for Bug 16420 to 
  events_bugs.test
sql/event_data_objects.cc:
  Move update_timing_fields functionality to Event_db_repository.
  Make loading of events from a table record more robust to tampering
  with the table - now we do not check mysql.event on every table open.
sql/event_data_objects.h:
  Cleanup.
sql/event_db_repository.cc:
  Now Event_db_repository is responsible for table I/O only.
  All the logic of events DDL is handled outside, in Events class please
  refer to the added test coverage to see how this change affected
  the behavior of Event Scheduler.
  Dependency on sp_head.h and sp.h removed.
  Make this module robust to tweaks with mysql.event table.
  Move check_system_tables from events.cc to this file
sql/event_db_repository.h:
  Cleanup declarations (remove unused ones, change return type to bool
  from int).
sql/event_queue.cc:
  Update to adapt to the new start up scheme of the Event Scheduler.
sql/event_queue.h:
  Cleanup declarations.
sql/event_scheduler.cc:
  Make all the error messages uniform:
  [SEVERITY] Event Scheduler: [user][schema.event] message
  Using append_identifier for error logging was an overkill - we may 
  need it only if the system character set may have NUL (null character)
  as part of a valid identifier, this is currently never the case, 
  whereas additional quoting did not look nice in the log.
sql/event_scheduler.h:
  Cleanup the headers.
sql/events.cc:
  Use a different start up procedure of Event Scheduler:
  - at start up, try to check the system tables first.
  If they are not up-to-date, disable the scheduler.
  - try to load all the active events. In case of a load error, abort
  start up.
  - do not parse an event on start up. Parsing only gives some information
  about event validity, but far not all.
  Consolidate the business logic of Events DDL in this module.
  Now opt_event_scheduler may change after start up and thus is protected
  by LOCK_event_metadata mutex.
sql/events.h:
  Use all-static-data-members approach to implement Singleton pattern.
sql/mysqld.cc:
  New invocation scheme of Events. Move some logic to events.cc.
  Initialize the scheduler before reporting "Ready for connections".
sql/set_var.cc:
  Clean up sys_var_thd_sql_mode::symbolic_mode_representation
  to work with a LEX_STRING.
  Move more logic related to @@events_scheduler global variable to Events
  module.
sql/set_var.h:
  Update declarations.
sql/share/errmsg.txt:
  If someone tampered with mysql.event table after the server has
  started we no longer give him/her a complete report what was actually 
  broken. Do not send the user to look at the error log in such case,
  as there is nothing there (check_table_intact is not executed).
sql/sp_head.cc:
  Update to a new declaration of 
  sys_var_thd_sql_mode::symbolic_mode_representation
sql/sql_db.cc:
  New invocation scheme of Events module.
sql/sql_parse.cc:
  Move more logic to Events module. Make sure that we are consistent
  in the way access rights are checked for Events DDL: always
  after committing the current transaction and checking the system tables.
sql/sql_show.cc:
  Update to the new declarations of 
  sys_var_thd_sql_mode::symbolic_mode_representation
sql/sql_test.cc:
  New invocation scheme of events.
sql/table.cc:
  mysql.event is a system table.
  Update check_table_intact to be concurrent, more verbose, and less smart.
sql/table.h:
  Add a helper method.
mysql-test/r/events_trans.result:
  New BitKeeper file ``mysql-test/r/events_trans.result''
mysql-test/t/events_trans.test:
  New BitKeeper file ``mysql-test/t/events_trans.test'':
  test cases for Event Scheduler that require a transactional
  storage engine.

1 files changed, 530 insertions, 422 deletions

diff --git a/sql/events.cc b/sql/events.cc
index fd8160e103c..3182cea25f2 100644
--- a/sql/events.cc
+++ b/sql/events.cc
@@ -19,7 +19,6 @@
 #include "event_db_repository.h"
 #include "event_queue.h"
 #include "event_scheduler.h"
-#include "sp_head.h"
 
 /*
  TODO list :
@@ -66,7 +65,7 @@
 static const char *opt_event_scheduler_state_names[]=
     { "OFF", "ON", "0", "1", "DISABLED", NullS };
 
-TYPELIB Events::opt_typelib=
+const TYPELIB Events::opt_typelib=
 {
   array_elements(opt_event_scheduler_state_names)-1,
   "",
@@ -82,7 +81,7 @@ TYPELIB Events::opt_typelib=
 */
 static const char *var_event_scheduler_state_names[]= { "OFF", "ON", NullS };
 
-TYPELIB Events::var_typelib=
+const TYPELIB Events::var_typelib=
 {
   array_elements(var_event_scheduler_state_names)-1,
   "",
@@ -90,20 +89,13 @@ TYPELIB Events::var_typelib=
   NULL
 };
 
-
-static
-Event_queue events_event_queue;
-
-static
-Event_scheduler events_event_scheduler;
-
-
-Event_db_repository events_event_db_repository;
-
-Events Events::singleton;
-
-enum Events::enum_opt_event_scheduler Events::opt_event_scheduler=
-     Events::EVENTS_OFF;
+Event_queue *Events::event_queue;
+Event_scheduler *Events::scheduler;
+Event_db_repository *Events::db_repository;
+enum Events::enum_opt_event_scheduler
+Events::opt_event_scheduler= Events::EVENTS_OFF;
+pthread_mutex_t Events::LOCK_event_metadata;
+bool Events::check_system_tables_error= FALSE;
 
 
 /*
@@ -128,25 +120,89 @@ int sortcmp_lex_string(LEX_STRING s, LEX_STRING t, CHARSET_INFO *cs)
 }
 
 
-/*
-  Accessor for the singleton instance.
+/**
+  @brief Initialize the start up option of the Events scheduler.
 
-  SYNOPSIS
-    Events::get_instance()
+  Do not initialize the scheduler subsystem yet - the initialization
+  is split into steps as it has to fit into the common MySQL
+  initialization framework.
+  No locking as this is called only at start up.
 
-  RETURN VALUE
-    address
+  @param[in,out]  argument  The value of the argument. If this value
+                            is found in the typelib, the argument is
+                            updated.
+
+  @retval TRUE  unknown option value
+  @retval FALSE success
 */
 
-Events *
-Events::get_instance()
+bool
+Events::set_opt_event_scheduler(char *argument)
 {
-  DBUG_ENTER("Events::get_instance");
-  DBUG_RETURN(&singleton);
+  if (argument == NULL)
+    opt_event_scheduler= Events::EVENTS_DISABLED;
+  else
+  {
+    int type;
+    /*
+      type=   1   2      3   4      5
+           (OFF | ON) - (0 | 1) (DISABLE )
+    */
+    const static enum enum_opt_event_scheduler type2state[]=
+    { EVENTS_OFF, EVENTS_ON, EVENTS_OFF, EVENTS_ON, EVENTS_DISABLED };
+
+    type= find_type(argument, &opt_typelib, 1);
+
+    DBUG_ASSERT(type >= 0 && type <= 5); /* guaranteed by find_type */
+
+    if (type == 0)
+    {
+      fprintf(stderr, "Unknown option to event-scheduler: %s\n", argument);
+      return TRUE;
+    }
+    opt_event_scheduler= type2state[type-1];
+  }
+  return FALSE;
 }
 
 
-/*
+/**
+  Return a string representation of the current scheduler mode.
+*/
+
+const char *
+Events::get_opt_event_scheduler_str()
+{
+  const char *str;
+
+  pthread_mutex_lock(&LOCK_event_metadata);
+  str= opt_typelib.type_names[(int) opt_event_scheduler];
+  pthread_mutex_unlock(&LOCK_event_metadata);
+
+  return str;
+}
+
+
+/**
+  Push an error into the error stack if the system tables are
+  not up to date.
+*/
+
+bool Events::check_if_system_tables_error()
+{
+  DBUG_ENTER("Events::check_if_system_tables_error");
+
+  if (check_system_tables_error)
+  {
+    my_error(ER_EVENTS_DB_ERROR, MYF(0));
+    DBUG_RETURN(TRUE);
+  }
+
+  DBUG_RETURN(FALSE);
+}
+
+
+/**
   Reconstructs interval expression from interval type and expression
   value that is in form of a value of the smalles entity:
   For
@@ -278,56 +334,70 @@ common_1_lev_code:
   return 0;
 }
 
-/*
-  Constructor of Events class. It's called when events.o
-  is loaded. Assigning addressed of static variables in this
-  object file.
 
-  SYNOPSIS
-    Events::Events()
+/**
+  Create a new event.
+
+  @param[in,out]  thd            THD
+  @param[in]      parse_data     Event's data from parsing stage
+  @param[in]      if_not_exists  Whether IF NOT EXISTS was
+                                 specified
+  In case there is an event with the same name (db) and
+  IF NOT EXISTS is specified, an warning is put into the stack.
+  @sa Events::drop_event for the notes about locking, pre-locking
+  and Events DDL.
+
+  @retval  FALSE  OK
+  @retval  TRUE   Error (reported)
 */
 
-Events::Events()
+bool
+Events::create_event(THD *thd, Event_parse_data *parse_data,
+                     bool if_not_exists)
 {
-  scheduler=     &events_event_scheduler;
-  event_queue=   &events_event_queue;
-  db_repository= &events_event_db_repository;
-}
+  int ret;
+  DBUG_ENTER("Events::create_event");
 
+  /*
+    Let's commit the transaction first - MySQL manual specifies
+    that a DDL issues an implicit commit, and it doesn't say "successful
+    DDL", so that an implicit commit is a property of any successfully
+    parsed DDL statement.
+  */
+  if (end_active_trans(thd))
+    DBUG_RETURN(TRUE);
 
-/*
-  The function exported to the world for creating of events.
+  if (check_if_system_tables_error())
+    DBUG_RETURN(TRUE);
 
-  SYNOPSIS
-    Events::create_event()
-      thd            [in]  THD
-      parse_data     [in]  Event's data from parsing stage
-      if_not_exists  [in]  Whether IF NOT EXISTS was specified in the DDL
+  /*
+    Perform semantic checks outside of Event_db_repository:
+    once CREATE EVENT is supported in prepared statements, the
+    checks will be moved to PREPARE phase.
+  */
+  if (parse_data->check_parse_data(thd))
+    DBUG_RETURN(TRUE);
 
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error (Reported)
+  /* At create, one of them must be set */
+  DBUG_ASSERT(parse_data->expression || parse_data->execute_at);
 
-  NOTES
-    In case there is an event with the same name (db) and
-    IF NOT EXISTS is specified, an warning is put into the stack.
-*/
+  if (check_access(thd, EVENT_ACL, parse_data->dbname.str, 0, 0, 0,
+                   is_schema_db(parse_data->dbname.str)))
+    DBUG_RETURN(TRUE);
 
-bool
-Events::create_event(THD *thd, Event_parse_data *parse_data, bool if_not_exists)
-{
-  int ret;
-  DBUG_ENTER("Events::create_event");
-  if (unlikely(check_system_tables_error))
+  if (check_db_dir_existence(parse_data->dbname.str))
   {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
+    my_error(ER_BAD_DB_ERROR, MYF(0));
     DBUG_RETURN(TRUE);
   }
 
+  if (parse_data->do_not_create)
+    DBUG_RETURN(FALSE);
+
   pthread_mutex_lock(&LOCK_event_metadata);
+
   /* On error conditions my_error() is called so no need to handle here */
-  if (!(ret= db_repository->create_event(thd, parse_data, if_not_exists)) &&
-      !parse_data->do_not_create)
+  if (!(ret= db_repository->create_event(thd, parse_data, if_not_exists)))
   {
     Event_queue_element *new_element;
 
@@ -337,11 +407,16 @@ Events::create_event(THD *thd, Event_parse_data *parse_data, bool if_not_exists)
                                                    parse_data->name,
                                                    new_element)))
     {
-      DBUG_ASSERT(ret == OP_LOAD_ERROR);
+      db_repository->drop_event(thd, parse_data->dbname, parse_data->name,
+                                TRUE);
       delete new_element;
     }
-    else
-      event_queue->create_event(thd, new_element);
+    else if (event_queue)
+    {
+      /* TODO: do not ignore the out parameter and a possible OOM error! */
+      bool created;
+      event_queue->create_event(thd, new_element, &created);
+    }
   }
   pthread_mutex_unlock(&LOCK_event_metadata);
 
@@ -349,42 +424,85 @@ Events::create_event(THD *thd, Event_parse_data *parse_data, bool if_not_exists)
 }
 
 
-/*
-  The function exported to the world for alteration of events.
-
-  SYNOPSIS
-    Events::update_event()
-      thd           [in]  THD
-      parse_data    [in]  Event's data from parsing stage
-      rename_to     [in]  Set in case of RENAME TO.
-
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error
-
-  NOTES
-    et contains data about dbname and event name.
-    new_name is the new name of the event, if not null this means
-    that RENAME TO was specified in the query
+/**
+  Alter an event.
+
+  @param[in,out] thd         THD
+  @param[in]     parse_data  Event's data from parsing stage
+  @param[in]     new_dbname  A new schema name for the event. Set in the case of
+                             ALTER EVENT RENAME, otherwise is NULL.
+  @param[in]     new_name    A new name for the event. Set in the case of
+                             ALTER EVENT RENAME
+
+  Parameter 'et' contains data about dbname and event name.
+  Parameter 'new_name' is the new name of the event, if not null
+  this means that RENAME TO was specified in the query
+  @sa Events::drop_event for the locking notes.
+
+  @retval  FALSE  OK
+  @retval  TRUE   error (reported)
 */
 
 bool
-Events::update_event(THD *thd, Event_parse_data *parse_data, sp_name *rename_to)
+Events::update_event(THD *thd, Event_parse_data *parse_data,
+                     LEX_STRING *new_dbname, LEX_STRING *new_name)
 {
   int ret;
   Event_queue_element *new_element;
+
   DBUG_ENTER("Events::update_event");
-  LEX_STRING *new_dbname= rename_to ? &rename_to->m_db : NULL;
-  LEX_STRING *new_name= rename_to ? &rename_to->m_name : NULL;
-  if (unlikely(check_system_tables_error))
-  {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
+
+  /*
+    For consistency, implicit COMMIT should be the first thing in the
+    execution chain.
+  */
+  if (end_active_trans(thd))
+    DBUG_RETURN(TRUE);
+
+  if (check_if_system_tables_error())
+    DBUG_RETURN(TRUE);
+
+  if (parse_data->check_parse_data(thd) || parse_data->do_not_create)
+    DBUG_RETURN(TRUE);
+
+  if (check_access(thd, EVENT_ACL, parse_data->dbname.str, 0, 0, 0,
+                   is_schema_db(parse_data->dbname.str)))
     DBUG_RETURN(TRUE);
+
+  if (new_dbname)                               /* It's a rename */
+  {
+    /* Check that the new and the old names differ. */
+    if ( !sortcmp_lex_string(parse_data->dbname, *new_dbname,
+                             system_charset_info) &&
+         !sortcmp_lex_string(parse_data->name, *new_name,
+                             system_charset_info))
+    {
+      my_error(ER_EVENT_SAME_NAME, MYF(0), parse_data->name.str);
+      DBUG_RETURN(TRUE);
+    }
+
+    /*
+      And the user has sufficient privileges to use the target database.
+      Do it before checking whether the database exists: we don't want
+      to tell the user that a database doesn't exist if they can not
+      access it.
+    */
+    if (check_access(thd, EVENT_ACL, new_dbname->str, 0, 0, 0,
+                     is_schema_db(new_dbname->str)))
+      DBUG_RETURN(TRUE);
+
+    /* Check that the target database exists */
+    if (check_db_dir_existence(new_dbname->str))
+    {
+      my_error(ER_BAD_DB_ERROR, MYF(0));
+      DBUG_RETURN(TRUE);
+    }
   }
 
   pthread_mutex_lock(&LOCK_event_metadata);
   /* On error conditions my_error() is called so no need to handle here */
-  if (!(ret= db_repository->update_event(thd, parse_data, new_dbname, new_name)))
+  if (!(ret= db_repository->update_event(thd, parse_data,
+                                         new_dbname, new_name)))
   {
     LEX_STRING dbname= new_dbname ? *new_dbname : parse_data->dbname;
     LEX_STRING name= new_name ? *new_name : parse_data->name;
@@ -397,9 +515,17 @@ Events::update_event(THD *thd, Event_parse_data *parse_data, sp_name *rename_to)
       DBUG_ASSERT(ret == OP_LOAD_ERROR);
       delete new_element;
     }
-    else
+    else if (event_queue)
+    {
+      /*
+        TODO: check if an update actually has inserted an entry
+        into the queue.
+        If not, and the element is ON COMPLETION NOT PRESERVE, delete
+        it right away.
+      */
       event_queue->update_event(thd, parse_data->dbname, parse_data->name,
                                 new_element);
+    }
   }
   pthread_mutex_unlock(&LOCK_event_metadata);
 
@@ -407,20 +533,28 @@ Events::update_event(THD *thd, Event_parse_data *parse_data, sp_name *rename_to)
 }
 
 
-/*
+/**
   Drops an event
 
-  SYNOPSIS
-    Events::drop_event()
-      thd             [in]  THD
-      dbname          [in]  Event's schema
-      name            [in]  Event's name
-      if_exists       [in]  When set and the event does not exist =>
-                            warning onto the stack
-
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error (reported)
+  @param[in,out]  thd        THD
+  @param[in]      dbname     Event's schema
+  @param[in]      name       Event's name
+  @param[in]      if_exists  When this is set and the event does not exist
+                             a warning is pushed into the warning stack.
+                             Otherwise the operation produces an error.
+
+  @note Similarly to DROP PROCEDURE, we do not allow DROP EVENT
+  under LOCK TABLES mode, unless table mysql.event is locked.  To
+  ensure that, we do not reset & backup the open tables state in
+  this function - if in LOCK TABLES or pre-locking mode, this will
+  lead to an error 'Table mysql.event is not locked with LOCK
+  TABLES' unless it _is_ locked. In pre-locked mode there is
+  another barrier - DROP EVENT commits the current transaction,
+  and COMMIT/ROLLBACK is not allowed in stored functions and
+  triggers.
+
+  @retval  FALSE  OK
+  @retval  TRUE   Error (reported)
 */
 
 bool
@@ -428,16 +562,35 @@ Events::drop_event(THD *thd, LEX_STRING dbname, LEX_STRING name, bool if_exists)
 {
   int ret;
   DBUG_ENTER("Events::drop_event");
-  if (unlikely(check_system_tables_error))
-  {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
+
+  /*
+    In MySQL, DDL must always commit: since mysql.* tables are
+    non-transactional, we must modify them outside a transaction
+    to not break atomicity.
+    But the second and more important reason to commit here
+    regardless whether we're actually changing mysql.event table
+    or not is replication: end_active_trans syncs the binary log,
+    and unless we run DDL in it's own transaction it may simply
+    never appear on the slave in case the outside transaction
+    rolls back.
+  */
+  if (end_active_trans(thd))
+    DBUG_RETURN(TRUE);
+
+  if (check_if_system_tables_error())
+    DBUG_RETURN(TRUE);
+
+  if (check_access(thd, EVENT_ACL, dbname.str, 0, 0, 0,
+                   is_schema_db(dbname.str)))
     DBUG_RETURN(TRUE);
-  }
 
   pthread_mutex_lock(&LOCK_event_metadata);
   /* On error conditions my_error() is called so no need to handle here */
   if (!(ret= db_repository->drop_event(thd, dbname, name, if_exists)))
-    event_queue->drop_event(thd, dbname, name);
+  {
+    if (event_queue)
+      event_queue->drop_event(thd, dbname, name);
+  }
   pthread_mutex_unlock(&LOCK_event_metadata);
   DBUG_RETURN(ret);
 }
@@ -446,10 +599,12 @@ Events::drop_event(THD *thd, LEX_STRING dbname, LEX_STRING name, bool if_exists)
 /**
   Drops all events from a schema
 
-  SYNOPSIS
-    Events::drop_schema_events()
-      thd  Thread
-      db   ASCIIZ schema name
+  @note We allow to drop all events in a schema even if the
+  scheduler is disabled. This is to not produce any warnings
+  in case of DROP DATABASE and a disabled scheduler.
+
+  @param[in,out]  thd  Thread
+  @param[in]      db   ASCIIZ schema name
 */
 
 void
@@ -459,14 +614,15 @@ Events::drop_schema_events(THD *thd, char *db)
 
   DBUG_ENTER("Events::drop_schema_events");
   DBUG_PRINT("enter", ("dropping events from %s", db));
-  if (unlikely(check_system_tables_error))
-  {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
-    DBUG_VOID_RETURN;
-  }
+
+  /*
+    sic: no check if the scheduler is disabled or system tables
+    are damaged, as intended.
+  */
 
   pthread_mutex_lock(&LOCK_event_metadata);
-  event_queue->drop_schema_events(thd, db_lex);
+  if (event_queue)
+    event_queue->drop_schema_events(thd, db_lex);
   db_repository->drop_schema_events(thd, db_lex);
   pthread_mutex_unlock(&LOCK_event_metadata);
 
@@ -474,115 +630,137 @@ Events::drop_schema_events(THD *thd, char *db)
 }
 
 
-/*
-  SHOW CREATE EVENT
-
-  SYNOPSIS
-    Events::show_create_event()
-      thd   Thread context
-      spn   The name of the event (db, name)
-
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error during writing to the wire
+/**
+  A helper function to generate SHOW CREATE EVENT output from
+  a named event
 */
 
-bool
-Events::show_create_event(THD *thd, LEX_STRING dbname, LEX_STRING name)
+static bool
+send_show_create_event(THD *thd, Event_timed *et, Protocol *protocol)
 {
-  CHARSET_INFO *scs= system_charset_info;
-  int ret;
-  Event_timed *et= new Event_timed();
+  char show_str_buf[10 * STRING_BUFFER_USUAL_SIZE];
+  String show_str(show_str_buf, sizeof(show_str_buf), system_charset_info);
+  List<Item> field_list;
+  LEX_STRING sql_mode;
+  const String *tz_name;
 
-  DBUG_ENTER("Events::show_create_event");
-  DBUG_PRINT("enter", ("name: %s@%s", dbname.str, name.str));
-  if (unlikely(check_system_tables_error))
-  {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
+  DBUG_ENTER("send_show_create_event");
+
+  show_str.length(0);
+  if (et->get_create_event(thd, &show_str))
     DBUG_RETURN(TRUE);
-  }
 
-  ret= db_repository->load_named_event(thd, dbname, name, et);
+  field_list.push_back(new Item_empty_string("Event", NAME_LEN));
 
-  if (!ret)
-  {
-    Protocol *protocol= thd->protocol;
-    char show_str_buf[10 * STRING_BUFFER_USUAL_SIZE];
-    String show_str(show_str_buf, sizeof(show_str_buf), scs);
-    List<Item> field_list;
-    byte *sql_mode_str;
-    ulong sql_mode_len=0;
+  if (sys_var_thd_sql_mode::symbolic_mode_representation(thd, et->sql_mode,
+                                                         &sql_mode))
+    DBUG_RETURN(TRUE);
 
-    show_str.length(0);
-    show_str.set_charset(system_charset_info);
+  field_list.push_back(new Item_empty_string("sql_mode", sql_mode.length));
 
-    if (et->get_create_event(thd, &show_str))
-      goto err;
+  tz_name= et->time_zone->get_name();
 
-    field_list.push_back(new Item_empty_string("Event", NAME_LEN));
+  field_list.push_back(new Item_empty_string("time_zone",
+                                             tz_name->length()));
 
-    sql_mode_str=
-      sys_var_thd_sql_mode::symbolic_mode_representation(thd, et->sql_mode,
-                                                         &sql_mode_len);
+  field_list.push_back(new Item_empty_string("Create Event",
+                                             show_str.length()));
 
-    field_list.push_back(new Item_empty_string("sql_mode", sql_mode_len));
+  if (protocol->send_fields(&field_list,
+                            Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF))
+    DBUG_RETURN(TRUE);
 
-    const String *tz_name= et->time_zone->get_name();
-    field_list.push_back(new Item_empty_string("time_zone",
-                                               tz_name->length()));
+  protocol->prepare_for_resend();
 
-    field_list.push_back(new Item_empty_string("Create Event",
-                                               show_str.length()));
+  protocol->store(et->name.str, et->name.length, system_charset_info);
+  protocol->store(sql_mode.str, sql_mode.length, system_charset_info);
+  protocol->store(tz_name->ptr(), tz_name->length(), system_charset_info);
+  protocol->store(show_str.c_ptr(), show_str.length(), system_charset_info);
 
-    if (protocol->send_fields(&field_list, Protocol::SEND_NUM_ROWS |
-                                           Protocol::SEND_EOF))
-      goto err;
+  if (protocol->write())
+    DBUG_RETURN(TRUE);
 
-    protocol->prepare_for_resend();
-    protocol->store(et->name.str, et->name.length, scs);
+  send_eof(thd);
 
-    protocol->store((char*) sql_mode_str, sql_mode_len, scs);
+  DBUG_RETURN(FALSE);
+}
 
-    protocol->store((char*) tz_name->ptr(), tz_name->length(), scs);
 
-    protocol->store(show_str.c_ptr(), show_str.length(), scs);
-    ret= protocol->write();
-    send_eof(thd);
-  }
-  delete et;
+/**
+  Implement SHOW CREATE EVENT statement
+
+      thd   Thread context
+      spn   The name of the event (db, name)
+
+  @retval  FALSE  OK
+  @retval  TRUE   error (reported)
+*/
+
+bool
+Events::show_create_event(THD *thd, LEX_STRING dbname, LEX_STRING name)
+{
+  Open_tables_state open_tables_backup;
+  Event_timed et;
+  bool ret;
+
+  DBUG_ENTER("Events::show_create_event");
+  DBUG_PRINT("enter", ("name: %s@%s", dbname.str, name.str));
+
+  if (check_if_system_tables_error())
+    DBUG_RETURN(TRUE);
+
+  if (check_access(thd, EVENT_ACL, dbname.str, 0, 0, 0,
+                   is_schema_db(dbname.str)))
+    DBUG_RETURN(TRUE);
+
+  /*
+    We would like to allow SHOW CREATE EVENT under LOCK TABLES and
+    in pre-locked mode. mysql.event table is marked as a system table.
+    This flag reduces the set of its participation scenarios in LOCK TABLES
+    operation, and therefore an out-of-bound open of this table
+    for reading like the one below (sic, only for reading) is
+    more or less deadlock-free. For additional information about when a
+    deadlock can occur please refer to the description of 'system table'
+    flag.
+  */
+  thd->reset_n_backup_open_tables_state(&open_tables_backup);
+  ret= db_repository->load_named_event(thd, dbname, name, &et);
+  thd->restore_backup_open_tables_state(&open_tables_backup);
+
+  if (!ret)
+    ret= send_show_create_event(thd, &et, thd->protocol);
+
   DBUG_RETURN(ret);
-err:
-  delete et;
-  DBUG_RETURN(TRUE);
 }
 
 
-/*
-  Proxy for Event_db_repository::fill_schema_events.
-  Callback for I_S from sql_show.cc
+/**
+  Check access rights and fill INFORMATION_SCHEMA.events table.
 
-  SYNOPSIS
-    Events::fill_schema_events()
-      thd     Thread context
-      tables  The schema table
+  @param[in,out]  thd     Thread context
+  @param[in]      table   The temporary table to fill.
       cond    Unused
 
-  RETURN VALUE
-    0  OK
-    !0 Error
+  In MySQL INFORMATION_SCHEMA tables are temporary tables that are
+  created and filled on demand. In this function, we fill
+  INFORMATION_SCHEMA.events. It is a callback for I_S module, invoked from
+  sql_show.cc
+
+  @return Has to be integer, as such is the requirement of the I_S API
+  @retval  0  success
+  @retval  1  an error, pushed into the error stack
 */
 
 int
 Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
 {
   char *db= NULL;
+  int ret;
+  Open_tables_state open_tables_backup;
   DBUG_ENTER("Events::fill_schema_events");
-  Events *myself= get_instance();
-  if (unlikely(myself->check_system_tables_error))
-  {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
-    DBUG_RETURN(TRUE);
-  }
+
+  if (check_if_system_tables_error())
+    DBUG_RETURN(1);
 
   /*
     If it's SHOW EVENTS then thd->lex->select_lex.db is guaranteed not to
@@ -596,7 +774,17 @@ Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
       DBUG_RETURN(1);
     db= thd->lex->select_lex.db;
   }
-  DBUG_RETURN(myself->db_repository->fill_schema_events(thd, tables, db));
+  /*
+    Reset and backup of the currently open tables in this thread
+    is a way to allow SELECTs from INFORMATION_SCHEMA.events under
+    LOCK TABLES and in pre-locked mode. See also
+    Events::show_create_event for additional comments.
+  */
+  thd->reset_n_backup_open_tables_state(&open_tables_backup);
+  ret= db_repository->fill_schema_events(thd, tables, db);
+  thd->restore_backup_open_tables_state(&open_tables_backup);
+
+  DBUG_RETURN(ret);
 }
 
 
@@ -615,14 +803,17 @@ Events::fill_schema_events(THD *thd, TABLE_LIST *tables, COND * /* cond */)
 */
 
 bool
-Events::init()
+Events::init(my_bool opt_noacl)
 {
   THD *thd;
   bool res= FALSE;
+
   DBUG_ENTER("Events::init");
 
-  if (opt_event_scheduler == Events::EVENTS_DISABLED)
-    DBUG_RETURN(FALSE);
+  /* Disable the scheduler if running with --skip-grant-tables */
+  if (opt_noacl)
+    opt_event_scheduler= EVENTS_DISABLED;
+
 
   /* We need a temporary THD during boot */
   if (!(thd= new THD()))
@@ -638,30 +829,67 @@ Events::init()
   thd->thread_stack= (char*) &thd;
   thd->store_globals();
 
-  if (check_system_tables(thd))
+  /*
+    We will need Event_db_repository anyway, even if the scheduler is
+    disabled - to perform events DDL.
+  */
+  if (!(db_repository= new Event_db_repository))
   {
-    check_system_tables_error= TRUE;
-    sql_print_error("Event Scheduler: The system tables are damaged. "
-                    "The scheduler subsystem will be unusable during this run.");
+    res= TRUE; /* fatal error: request unireg_abort */
     goto end;
   }
-  check_system_tables_error= FALSE;
 
-  if (event_queue->init_queue(thd) || load_events_from_db(thd))
+  /*
+    Since we allow event DDL even if the scheduler is disabled,
+    check the system tables, as we might need them.
+  */
+  if (Event_db_repository::check_system_tables(thd))
   {
-    sql_print_error("Event Scheduler: Error while loading from disk.");
+    sql_print_error("Event Scheduler: An error occurred when initializing "
+                    "system tables.%s",
+                    opt_event_scheduler == EVENTS_DISABLED ?
+                    "" : " Disabling the Event Scheduler.");
+
+    /* Disable the scheduler since the system tables are not up to date */
+    opt_event_scheduler= EVENTS_DISABLED;
+    check_system_tables_error= TRUE;
     goto end;
   }
 
-  scheduler->init_scheduler(event_queue);
+  /*
+    Was disabled explicitly from the command line, or because we're running
+    with --skip-grant-tables, 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);
-  if (opt_event_scheduler == Events::EVENTS_ON)
-    res= scheduler->start();
 
-  Event_worker_thread::init(this, db_repository);
+  if (!(event_queue= new Event_queue) ||
+      !(scheduler= new Event_scheduler(event_queue)))
+  {
+    res= TRUE; /* fatal error: request unireg_abort */
+    goto end;
+  }
+
+  if (event_queue->init_queue(thd) || load_events_from_db(thd) ||
+      opt_event_scheduler == EVENTS_ON && scheduler->start())
+  {
+    sql_print_error("Event Scheduler: Error while loading from disk.");
+    res= TRUE; /* fatal error: request unireg_abort */
+    goto end;
+  }
+  Event_worker_thread::init(db_repository);
+
 end:
+  if (res)
+  {
+    delete db_repository;
+    delete event_queue;
+    delete scheduler;
+  }
   delete thd;
   /* Remember that we don't have a THD */
   my_pthread_setspecific_ptr(THR_THD,  NULL);
@@ -684,14 +912,18 @@ void
 Events::deinit()
 {
   DBUG_ENTER("Events::deinit");
-  if (likely(!check_system_tables_error))
-  {
-    scheduler->stop();
-    scheduler->deinit_scheduler();
 
-    event_queue->deinit_queue();
+  if (opt_event_scheduler != EVENTS_DISABLED)
+  {
+    delete scheduler;
+    scheduler= NULL;                            /* safety */
+    delete event_queue;
+    event_queue= NULL;                          /* safety */
   }
 
+  delete db_repository;
+  db_repository= NULL;                          /* safety */
+
   DBUG_VOID_RETURN;
 }
 
@@ -708,8 +940,6 @@ void
 Events::init_mutexes()
 {
   pthread_mutex_init(&LOCK_event_metadata, MY_MUTEX_INIT_FAST);
-  event_queue->init_mutexes();
-  scheduler->init_mutexes();
 }
 
 
@@ -723,8 +953,6 @@ Events::init_mutexes()
 void
 Events::destroy_mutexes()
 {
-  event_queue->deinit_mutexes();
-  scheduler->deinit_mutexes();
   pthread_mutex_destroy(&LOCK_event_metadata);
 }
 
@@ -747,283 +975,163 @@ Events::dump_internal_status()
   puts("LLA = Last Locked At  LUA = Last Unlocked At");
   puts("WOC = Waiting On Condition  DL = Data Locked");
 
-  scheduler->dump_internal_status();
-  event_queue->dump_internal_status();
+  pthread_mutex_lock(&LOCK_event_metadata);
+  if (opt_event_scheduler == EVENTS_DISABLED)
+    puts("The Event Scheduler is disabled");
+  else
+  {
+    scheduler->dump_internal_status();
+    event_queue->dump_internal_status();
+  }
 
+  pthread_mutex_unlock(&LOCK_event_metadata);
   DBUG_VOID_RETURN;
 }
 
 
 /**
-  Starts execution of events by the scheduler
+  Starts or stops the event scheduler thread.
 
-  SYNOPSIS
-    Events::start_execution_of_events()
-
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error
+  @retval FALSE success
+  @retval TRUE  error
 */
 
 bool
-Events::start_execution_of_events()
+Events::start_or_stop_event_scheduler(enum_opt_event_scheduler start_or_stop)
 {
-  DBUG_ENTER("Events::start_execution_of_events");
-  if (unlikely(check_system_tables_error))
-  {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
-    DBUG_RETURN(TRUE);
-  }
-  DBUG_RETURN(scheduler->start());
-}
-
-
-/*
-  Stops execution of events by the scheduler.
-  Already running events will not be stopped. If the user needs
-  them stopped manual intervention is needed.
+  bool ret= FALSE;
 
-  SYNOPSIS
-    Events::stop_execution_of_events()
+  DBUG_ENTER("Events::start_or_stop_event_scheduler");
 
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error
-*/
+  DBUG_ASSERT(start_or_stop == Events::EVENTS_ON ||
+              start_or_stop == Events::EVENTS_OFF);
 
-bool
-Events::stop_execution_of_events()
-{
-  DBUG_ENTER("Events::stop_execution_of_events");
-  if (unlikely(check_system_tables_error))
-  {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
+  /*
+    If the scheduler was disabled because there are no/bad
+    system tables, produce a more meaningful error message
+    than ER_OPTION_PREVENTS_STATEMENT
+  */
+  if (check_if_system_tables_error())
     DBUG_RETURN(TRUE);
-  }
-  DBUG_RETURN(scheduler->stop());
-}
-
-
-/*
-  Checks whether the scheduler is running or not.
-
-  SYNOPSIS
-    Events::is_started()
-
-  RETURN VALUE
-    TRUE   Yes
-    FALSE  No
-*/
 
-bool
-Events::is_execution_of_events_started()
-{
-  DBUG_ENTER("Events::is_execution_of_events_started");
-  if (unlikely(check_system_tables_error))
-  {
-    my_error(ER_EVENTS_DB_ERROR, MYF(0));
-    DBUG_RETURN(FALSE);
-  }
-  DBUG_RETURN(scheduler->is_running());
-}
-
-
-
-/*
-  Opens mysql.db and mysql.user and checks whether:
-    1. mysql.db has column Event_priv at column 20 (0 based);
-    2. mysql.user has column Event_priv at column 29 (0 based);
-
-  SYNOPSIS
-    Events::check_system_tables()
-      thd  Thread
-
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error
-*/
-
-bool
-Events::check_system_tables(THD *thd)
-{
-  TABLE_LIST tables;
-  Open_tables_state backup;
-  bool ret= FALSE;
-
-  DBUG_ENTER("Events::check_system_tables");
-  DBUG_PRINT("enter", ("thd: 0x%lx", (long) thd));
-
-  thd->reset_n_backup_open_tables_state(&backup);
-
-  bzero((char*) &tables, sizeof(tables));
-  tables.db= (char*) "mysql";
-  tables.table_name= tables.alias= (char*) "db";
-  tables.lock_type= TL_READ;
+  pthread_mutex_lock(&LOCK_event_metadata);
 
-  if ((ret= simple_open_n_lock_tables(thd, &tables)))
+  if (opt_event_scheduler == EVENTS_DISABLED)
   {
-    sql_print_error("Event Scheduler: Cannot open mysql.db");
+    my_error(ER_OPTION_PREVENTS_STATEMENT,
+             MYF(0), "--event-scheduler=DISABLED or --skip-grant-tables");
     ret= TRUE;
+    goto end;
   }
-  ret= table_check_intact(tables.table, MYSQL_DB_FIELD_COUNT,
-                          mysql_db_table_fields, &mysql_db_table_last_check,
-                          ER_CANNOT_LOAD_FROM_TABLE);
-  close_thread_tables(thd);
-
-  bzero((char*) &tables, sizeof(tables));
-  tables.db= (char*) "mysql";
-  tables.table_name= tables.alias= (char*) "user";
-  tables.lock_type= TL_READ;
 
-  if (simple_open_n_lock_tables(thd, &tables))
-  {
-    sql_print_error("Event Scheduler: Cannot open mysql.user");
-    ret= TRUE;
-  }
+  if (start_or_stop == EVENTS_ON)
+    ret= scheduler->start();
   else
+    ret= scheduler->stop();
+
+  if (ret)
   {
-    if (tables.table->s->fields < 29 ||
-        strncmp(tables.table->field[29]->field_name,
-                STRING_WITH_LEN("Event_priv")))
-    {
-      sql_print_error("mysql.user has no `Event_priv` column at position %d",
-                      29);
-      ret= TRUE;
-    }
-    close_thread_tables(thd);
+    my_error(ER_EVENT_SET_VAR_ERROR, MYF(0));
+    goto end;
   }
 
-  thd->restore_backup_open_tables_state(&backup);
+  opt_event_scheduler= start_or_stop;
 
+end:
+  pthread_mutex_unlock(&LOCK_event_metadata);
   DBUG_RETURN(ret);
 }
 
 
-/*
-  Loads all ENABLED events from mysql.event into the prioritized
-  queue. Called during scheduler main thread initialization. Compiles
-  the events. Creates Event_queue_element instances for every ENABLED event
-  from mysql.event.
+/**
+  Loads all ENABLED events from mysql.event into a prioritized
+  queue.
 
-  SYNOPSIS
-    Events::load_events_from_db()
-      thd  Thread context. Used for memory allocation in some cases.
+  This function is called during the server start up. It reads
+  every event, computes the next execution time, and if the event
+  needs execution, adds it to a prioritized queue. Otherwise, if
+  ON COMPLETION DROP is specified, the event is automatically
+  removed from the table.
 
-  RETURN VALUE
-    0  OK
-   !0  Error (EVEX_OPEN_TABLE_FAILED, EVEX_MICROSECOND_UNSUP,
-              EVEX_COMPILE_ERROR) - in all these cases mysql.event was
-              tampered.
+  @param[in,out] thd Thread context. Used for memory allocation in some cases.
 
-  NOTES
-    Reports the error to the console
+  @retval  FALSE  success
+  @retval  TRUE   error, the load is aborted
+
+  @note Reports the error to the console
 */
 
-int
+bool
 Events::load_events_from_db(THD *thd)
 {
   TABLE *table;
   READ_RECORD read_record_info;
-  int ret= -1;
+  bool ret= TRUE;
   uint count= 0;
-  bool clean_the_queue= TRUE;
 
   DBUG_ENTER("Events::load_events_from_db");
   DBUG_PRINT("enter", ("thd: 0x%lx", (long) thd));
 
-  if ((ret= db_repository->open_event_table(thd, TL_READ, &table)))
+  if ((ret= db_repository->open_event_table(thd, TL_WRITE, &table)))
   {
-    sql_print_error("Event Scheduler: Table mysql.event is damaged. Can not open");
-    DBUG_RETURN(EVEX_OPEN_TABLE_FAILED);
+    sql_print_error("Event Scheduler: Failed to open table mysql.event");
+    DBUG_RETURN(TRUE);
   }
 
-  init_read_record(&read_record_info, thd, table ,NULL,1,0);
+  init_read_record(&read_record_info, thd, table, NULL, 0, 1);
   while (!(read_record_info.read_record(&read_record_info)))
   {
     Event_queue_element *et;
+    bool created;
+    bool drop_on_completion;
+
     if (!(et= new Event_queue_element))
-    {
-      DBUG_PRINT("info", ("Out of memory"));
-      break;
-    }
+      goto end;
+
     DBUG_PRINT("info", ("Loading event from row."));
 
-    if ((ret= et->load_from_row(thd, table)))
+    if (et->load_from_row(thd, table))
     {
       sql_print_error("Event Scheduler: "
                       "Error while reading from mysql.event. "
                       "The table is probably corrupted");
-      break;
-    }
-    if (et->status != Event_queue_element::ENABLED)
-    {
-      DBUG_PRINT("info",("%s is disabled",et->name.str));
       delete et;
-      continue;
+      goto end;
     }
+    drop_on_completion= (et->on_completion ==
+                         Event_queue_element::ON_COMPLETION_DROP);
 
-    /* let's find when to be executed */
-    if (et->compute_next_execution_time())
+
+    if (event_queue->create_event(thd, et, &created))
     {
-      sql_print_error("Event Scheduler: Error while computing execution time of %s.%s."
-                      " Skipping", et->dbname.str, et->name.str);
-      continue;
+      /* Out of memory */
+      delete et;
+      goto end;
     }
-
+    if (created)
+      count++;
+    else if (drop_on_completion)
     {
-      Event_job_data temp_job_data;
-      DBUG_PRINT("info", ("Event %s loaded from row. ", et->name.str));
-
-      temp_job_data.load_from_row(thd, table);
-
       /*
-        We load only on scheduler root just to check whether the body
-        compiles.
+        If not created, a stale event - drop if immediately if
+        ON COMPLETION NOT PRESERVE
       */
-      switch (ret= temp_job_data.compile(thd, thd->mem_root)) {
-      case EVEX_MICROSECOND_UNSUP:
-        sql_print_error("Event Scheduler: mysql.event is tampered. MICROSECOND is not "
-                        "supported but found in mysql.event");
-        break;
-      case EVEX_COMPILE_ERROR:
-        sql_print_error("Event Scheduler: Error while compiling %s.%s. Aborting load",
-                        et->dbname.str, et->name.str);
-        break;
-      default:
-        break;
+      int rc= table->file->ha_delete_row(table->record[0]);
+      if (rc)
+      {
+        table->file->print_error(rc, MYF(0));
+        goto end;
       }
-      thd->end_statement();
-      thd->cleanup_after_query();
     }
-    if (ret)
-    {
-      delete et;
-      goto end;
-    }
-
-    DBUG_PRINT("load_events_from_db", ("Adding 0x%lx to the exec list.",
-                                       (long) et));
-    event_queue->create_event(thd, et);
-    count++;
   }
-  clean_the_queue= FALSE;
+  sql_print_information("Event Scheduler: Loaded %d event%s",
+                        count, (count == 1) ? "" : "s");
+  ret= FALSE;
+
 end:
   end_read_record(&read_record_info);
 
-  if (clean_the_queue)
-  {
-    event_queue->empty_queue();
-    ret= -1;
-  }
-  else
-  {
-    ret= 0;
-    sql_print_information("Event Scheduler: Loaded %d event%s",
-                          count, (count == 1)?"":"s");
-  }
-
   close_thread_tables(thd);
 
-  DBUG_PRINT("info", ("Status code %d. Loaded %d event(s)", ret, count));
   DBUG_RETURN(ret);
 }