MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:konstantin Date:April 13 2007 8:32pm
Subject:bk commit into 5.1 tree (kostja:1.2563) BUG#27733
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of kostja. When kostja does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-04-13 16:32:31-04:00, kostja@vajra.(none) +8 -0
  An attempt to fix a sporadic valgrind memory leak in Event Scheduler:
  streamline the event worker thread work flow and try to eliminate
  possibilities for memory corruptions that might have been
  lurking in previous (complicated) code.
  This patch: 
   * removes Event_job_data::compile that is now never used
   * cleans up Event_job_data::execute to minimize juggling with
     thread context and eliminate unneded code paths
   * Implements Security_context::change/restore_security_context
     to be able to re-use these methods in all stored programs
  This is to maybe fix Bug#27733 "Valgrind failures in 
  remove_table_from_cache".
  Review comments applied.

  sql/event_data_objects.cc@stripped, 2007-04-13 16:32:25-04:00, kostja@vajra.(none) +156 -274
    Remove Event_job_data::compile, which was never used without
    Event_job_data::execute().
    Merge the implementation of compile() with Event_job_data::execute().
    Reuse existing functions to prepare the event worker thread
    for execution instead of some previously copy-pasted code.
    Do not change and restore the current database inside 
    Event_job_data::execute(), just set the current database in the 
    thread, that is enough to parse and execute an event.

  sql/event_data_objects.h@stripped, 2007-04-13 16:32:25-04:00, kostja@vajra.(none) +3 -10
    Update declarations.

  sql/event_scheduler.cc@stripped, 2007-04-13 16:32:25-04:00, kostja@vajra.(none) +18 -36
    Allocate Event_job_data on stack.

  sql/item_func.cc@stripped, 2007-04-13 16:32:26-04:00, kostja@vajra.(none) +1 -1
    Update to match the new declaration of restore_security_context()

  sql/sp_head.cc@stripped, 2007-04-13 16:32:26-04:00, kostja@vajra.(none) +8 -42
    Update to match the new declaration of 
    change/restore_security_context()

  sql/sql_class.cc@stripped, 2007-04-13 16:32:26-04:00, kostja@vajra.(none) +96 -0
    Move change/restore_security_context to class Security_context.
    Add more comments.

  sql/sql_class.h@stripped, 2007-04-13 16:32:26-04:00, kostja@vajra.(none) +12 -0
    Make change/restore_security_context methods of Security_context.
    That allows us to reuse them in Event Scheduler (instead of a
    copy-paste presently used there).

  sql/sql_trigger.cc@stripped, 2007-04-13 16:32:26-04:00, kostja@vajra.(none) +9 -3
    Update to match the new declaration of 
    change/restore_security_context()

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	kostja
# Host:	vajra.(none)
# Root:	/opt/local/work/mysql-5.1-c1

--- 1.375/sql/item_func.cc	2007-04-05 05:17:46 -04:00
+++ 1.376/sql/item_func.cc	2007-04-13 16:32:26 -04:00
@@ -5351,7 +5351,7 @@
     Security_context *save_secutiry_ctx;
     res= set_routine_security_ctx(thd, m_sp, false, &save_secutiry_ctx);
     if (!res)
-      sp_restore_security_context(thd, save_secutiry_ctx);
+      m_sp->m_security_ctx.restore_security_context(thd, save_secutiry_ctx);
     
 #endif /* ! NO_EMBEDDED_ACCESS_CHECKS */
   }

--- 1.325/sql/sql_class.cc	2007-03-29 07:52:07 -04:00
+++ 1.326/sql/sql_class.cc	2007-04-13 16:32:26 -04:00
@@ -2119,6 +2119,102 @@
   return user == 0;
 }
 
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+/**
+  Initialize this security context from the passed in credentials
+  and activate it in the current thread.
+
+  @param[out]  backup  Save a pointer to the current security context
+                       in the thread. In case of success it points to the
+                       saved old context, otherwise it points to NULL.
+
+
+  During execution of a statement, multiple security contexts may
+  be needed:
+  - the security context of the authenticated user, used as the
+    default security context for all top-level statements
+  - in case of a view or a stored program, possibly the security
+    context of the definer of the routine, if the object is
+    defined with SQL SECURITY DEFINER option.
+
+  The currently "active" security context is parameterized in THD
+  member security_ctx. By default, after a connection is
+  established, this member points at the "main" security context
+  - the credentials of the authenticated user.
+
+  Later, if we would like to execute some sub-statement or a part
+  of a statement under credentials of a different user, e.g.
+  definer of a procedure, we authenticate this user in a local
+  instance of Security_context by means of this method (and
+  ultimately by means of acl_getroot_no_password), and make the
+  local instance active in the thread by re-setting
+  thd->security_ctx pointer.
+
+  Note, that the life cycle and memory management of the "main" and
+  temporary security contexts are different.
+  For the main security context, the memory for user/host/ip is
+  allocated on system heap, and the THD class frees this memory in
+  its destructor. The only case when contents of the main security
+  context may change during its life time is when someone issued
+  CHANGE USER command.
+  Memory management of a "temporary" security context is
+  responsibility of the module that creates it.
+
+  @retval TRUE  there is no user with the given credentials. The erro
+                is reported in the thread.
+  @retval FALSE success
+*/
+
+bool
+Security_context::
+change_security_context(THD *thd,
+                        LEX_STRING *definer_user,
+                        LEX_STRING *definer_host,
+                        LEX_STRING *db,
+                        Security_context **backup)
+{
+  bool needs_change;
+
+  DBUG_ENTER("Security_context::change_security_context");
+
+  DBUG_ASSERT(definer_user->str && definer_host->str);
+
+  *backup= NULL;
+  /*
+    The current security context may have NULL members
+    if we have just started the thread and not authenticated
+    any user. This use case is currently in events worker thread.
+  */
+  needs_change= (thd->security_ctx->priv_user == NULL ||
+                 strcmp(definer_user->str, thd->security_ctx->priv_user) ||
+                 thd->security_ctx->priv_host == NULL ||
+                 my_strcasecmp(system_charset_info, definer_host->str,
+                               thd->security_ctx->priv_host));
+  if (needs_change)
+  {
+    if (acl_getroot_no_password(this, definer_user->str, definer_host->str,
+                                definer_host->str, db->str))
+    {
+      my_error(ER_NO_SUCH_USER, MYF(0), definer_user->str,
+               definer_host->str);
+      DBUG_RETURN(TRUE);
+    }
+    *backup= thd->security_ctx;
+    thd->security_ctx= this;
+  }
+
+  DBUG_RETURN(FALSE);
+}
+
+
+void
+Security_context::restore_security_context(THD *thd,
+                                           Security_context *backup)
+{
+  if (backup)
+    thd->security_ctx= backup;
+}
+#endif
 
 /****************************************************************************
   Handling of open and locked tables states.

--- 1.351/sql/sql_class.h	2007-03-31 06:38:12 -04:00
+++ 1.352/sql/sql_class.h	2007-04-13 16:32:26 -04:00
@@ -656,6 +656,18 @@
   }
   
   bool set_user(char *user_arg);
+
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+  bool
+  change_security_context(THD *thd,
+                          LEX_STRING *definer_user,
+                          LEX_STRING *definer_host,
+                          LEX_STRING *db,
+                          Security_context **backup);
+
+  void
+  restore_security_context(THD *thd, Security_context *backup);
+#endif
 };
 
 

--- 1.42/sql/event_scheduler.cc	2007-04-06 11:44:08 -04:00
+++ 1.43/sql/event_scheduler.cc	2007-04-13 16:32:25 -04:00
@@ -277,8 +277,7 @@
 {
   /* needs to be first for thread_stack */
   char my_stack;
-  int ret;
-  Event_job_data *job_data= NULL;
+  Event_job_data job_data;
   bool res;
 
   thd->thread_stack= &my_stack;                // remember where our stack is
@@ -291,60 +290,43 @@
   if (res)
     goto end;
 
-  if (!(job_data= new Event_job_data()))
-    goto end;
-  else if ((ret= db_repository->
-                  load_named_event(thd, event->dbname, event->name, job_data)))
+  if ((res= db_repository->load_named_event(thd, event->dbname, event->name,
+                                            &job_data)))
   {
-    DBUG_PRINT("error", ("Got %d from load_named_event", ret));
+    DBUG_PRINT("error", ("Got error from load_named_event"));
     goto end;
   }
 
   sql_print_information("Event Scheduler: "
-                        "[%s.%s of %s] executing in thread %lu. ",
-                        job_data->dbname.str, job_data->name.str,
-                        job_data->definer.str, thd->thread_id);
+                        "[%s].[%s.%s] started in thread %lu.",
+                        job_data.definer.str,
+                        job_data.dbname.str, job_data.name.str,
+                        thd->thread_id);
 
   thd->enable_slow_log= TRUE;
 
-  ret= job_data->execute(thd, event->dropped);
+  res= job_data.execute(thd, event->dropped);
 
-  print_warnings(thd, job_data);
+  print_warnings(thd, &job_data);
 
-  switch (ret) {
-  case 0:
+  if (res)
+    sql_print_information("Event Scheduler: "
+                          "[%s].[%s.%s] event execution failed.",
+                          job_data.definer.str,
+                          job_data.dbname.str, job_data.name.str);
+  else
     sql_print_information("Event Scheduler: "
                           "[%s].[%s.%s] executed successfully in thread %lu.",
-                          job_data->definer.str,
-                          job_data->dbname.str, job_data->name.str,
+                          job_data.definer.str,
+                          job_data.dbname.str, job_data.name.str,
                           thd->thread_id);
-    break;
-  case EVEX_COMPILE_ERROR:
-    sql_print_information("Event Scheduler: "
-                          "[%s].[%s.%s] event compilation failed.",
-                          job_data->definer.str,
-                          job_data->dbname.str, job_data->name.str);
-    break;
-  default:
-    sql_print_information("Event Scheduler: "
-                          "[%s].[%s.%s] event execution failed.",
-                          job_data->definer.str,
-                          job_data->dbname.str, job_data->name.str);
-    break;
-  }
 
 end:
-  delete job_data;
-
   DBUG_PRINT("info", ("Done with Event %s.%s", event->dbname.str,
              event->name.str));
 
   delete event;
   deinit_event_thread(thd);
-  /*
-    Do not pthread_exit since we want local destructors for stack objects
-    to be invoked.
-  */
 }
 
 

--- 1.96/sql/event_data_objects.cc	2007-04-05 17:53:09 -04:00
+++ 1.97/sql/event_data_objects.cc	2007-04-13 16:32:25 -04:00
@@ -23,14 +23,6 @@
 
 #define EVEX_MAX_INTERVAL_VALUE 1000000000L
 
-static bool
-event_change_security_context(THD *thd, LEX_STRING user, LEX_STRING host,
-                              LEX_STRING db, Security_context *backup);
-
-static void
-event_restore_security_context(THD *thd, Security_context *backup);
-
-
 /*
   Initiliazes dbname and name of an Event_queue_element_for_exec
   object
@@ -816,27 +808,10 @@
 */
 
 Event_job_data::Event_job_data()
-  :sphead(NULL), sql_mode(0)
+  :sql_mode(0)
 {
 }
 
-
-/*
-  Destructor
-
-  SYNOPSIS
-    Event_timed::~Event_timed()
-*/
-
-Event_job_data::~Event_job_data()
-{
-  DBUG_ENTER("Event_job_data::~Event_job_data");
-  delete sphead;
-  sphead= NULL;
-  DBUG_VOID_RETURN;
-}
-
-
 /*
   Init all member variables
 
@@ -1769,234 +1744,202 @@
 }
 
 
-/*
-  Get SHOW CREATE EVENT as string
-
-  SYNOPSIS
-    Event_job_data::get_create_event(THD *thd, String *buf)
-      thd    Thread
-      buf    String*, should be already allocated. CREATE EVENT goes inside.
-
-  RETURN VALUE
-    0                       OK
-    EVEX_MICROSECOND_UNSUP  Error (for now if mysql.event has been
-                            tampered and MICROSECONDS interval or
-                            derivative has been put there.
+/**
+  Get an artificial stored procedure to parse as an event definition.
 */
 
-int
-Event_job_data::get_fake_create_event(String *buf)
+bool
+Event_job_data::construct_sp_sql(THD *thd, String *sp_sql)
 {
-  DBUG_ENTER("Event_job_data::get_create_event");
-  /* FIXME: "EVERY 3337 HOUR" is asking for trouble. */
-  buf->append(STRING_WITH_LEN("CREATE EVENT anonymous ON SCHEDULE "
-                              "EVERY 3337 HOUR DO "));
-  buf->append(body.str, body.length);
+  LEX_STRING buffer;
+  const uint STATIC_SQL_LENGTH= 44;
 
-  DBUG_RETURN(0);
+  DBUG_ENTER("Event_job_data::construct_sp_sql");
+
+  /*
+    Allocate a large enough buffer on the thread execution memory
+    root to avoid multiple [re]allocations on system heap
+  */
+  buffer.length= STATIC_SQL_LENGTH + name.length + body.length;
+  if (! (buffer.str= (char*) thd->alloc(buffer.length)))
+    DBUG_RETURN(TRUE);
+
+  sp_sql->set(buffer.str, buffer.length, system_charset_info);
+  sp_sql->length(0);
+
+
+  sp_sql->append(C_STRING_WITH_LEN("CREATE "));
+  sp_sql->append(C_STRING_WITH_LEN("PROCEDURE "));
+  /*
+    Let's use the same name as the event name to perhaps produce a
+    better error message in case it is a part of some parse error.
+    We're using append_identifier here to successfully parse
+    events with reserved names.
+  */
+  append_identifier(thd, sp_sql, name.str, name.length);
+
+  /*
+    The default SQL security of a stored procedure is DEFINER. We
+    have already activated the security context of the event, so
+    let's execute the procedure with the invoker rights to save on
+    resets of security contexts.
+  */
+  sp_sql->append(C_STRING_WITH_LEN("() SQL SECURITY INVOKER "));
+
+  sp_sql->append(body.str, body.length);
+
+  DBUG_RETURN(thd->is_fatal_error);
 }
 
 
-/*
-  Executes the event (the underlying sp_head object);
 
-  SYNOPSIS
-    Event_job_data::execute()
-      thd       THD
+/**
+  Compiles and executes the event (the underlying sp_head object)
 
-  RETURN VALUE
-    0        success
-    -99      No rights on this.dbname.str
-    others   retcodes of sp_head::execute_procedure()
+  @retval TRUE  error (reported to the error log)
+  @retval FALSE success
 */
 
-int
+bool
 Event_job_data::execute(THD *thd, bool drop)
 {
-  Security_context save_ctx;
-  /* this one is local and not needed after exec */
-  int ret= 0;
+  String sp_sql;
+  Security_context event_sctx, *save_sctx= NULL;
+  CHARSET_INFO *charset_connection;
+  List<Item> empty_item_list;
+  bool ret= TRUE;
 
   DBUG_ENTER("Event_job_data::execute");
-  DBUG_PRINT("info", ("EXECUTING %s.%s", dbname.str, name.str));
 
-  if ((ret= compile(thd, NULL)))
-    goto done;
+  mysql_reset_thd_for_next_command(thd);
 
-  event_change_security_context(thd, definer_user, definer_host, dbname,
-                                &save_ctx);
   /*
-    THD::~THD will clean this or if there is DROP DATABASE in the
-    SP then it will be free there. It should not point to our buffer
-    which is allocated on a mem_root.
+    MySQL parser currently assumes that current database is either
+    present in THD or all names in all statements are fully specified.
+    And yet not fully specified names inside stored programs must be 
+    be supported, even if the current database is not set:
+    CREATE PROCEDURE db1.p1() BEGIN CREATE TABLE t1; END//
+    -- in this example t1 should be always created in db1 and the statement
+    must parse even if there is no current database.
+
+    To support this feature and still address the parser limitation,
+    we need to set the current database here.
+    We don't have to call mysql_change_db, since the checks performed
+    in it are unnecessary for the purpose of parsing, and
+    mysql_change_db will be invoked anyway later, to activate the
+    procedure database before it's executed.
   */
-  thd->db= my_strdup(dbname.str, MYF(0));
-  thd->db_length= dbname.length;
-  if (!check_access(thd, EVENT_ACL,dbname.str, 0, 0, 0,is_schema_db(dbname.str)))
-  {
-    List<Item> empty_item_list;
-    empty_item_list.empty();
-    if (thd->enable_slow_log)
-      sphead->m_flags|= sp_head::LOG_SLOW_STATEMENTS;
-    sphead->m_flags|= sp_head::LOG_GENERAL_LOG;
-
-    /* Execute the event in its time zone. */
-    thd->variables.time_zone= time_zone;
+  thd->set_db(dbname.str, dbname.length);
 
-    ret= sphead->execute_procedure(thd, &empty_item_list);
-  }
-  else
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+  if (event_sctx.change_security_context(thd,
+                                         &definer_user, &definer_host,
+                                         &dbname, &save_sctx))
   {
-    DBUG_PRINT("error", ("%s@%s has no rights on %s", definer_user.str,
-               definer_host.str, dbname.str));
-    ret= -99;
+    sql_print_error("Event Scheduler: "
+                    "[%s].[%s.%s] execution failed, "
+                    "failed to authenticate the user.",
+                    definer.str, dbname.str, name.str);
+    goto end;
   }
-  if (drop)
+#endif
+
+  if (check_access(thd, EVENT_ACL, dbname.str,
+                   0, 0, 0, is_schema_db(dbname.str)))
   {
-    sql_print_information("Event Scheduler: Dropping %s.%s",
-                          dbname.str, name.str);
     /*
-      We must do it here since here we're under the right authentication
-      ID of the event definer
+      This aspect of behavior is defined in the worklog,
+      and this is how triggers work too: if TRIGGER
+      privilege is revoked from trigger definer,
+      triggers are not executed.
     */
-    if (Events::drop_event(thd, dbname, name, FALSE))
-      ret= 1;
+    sql_print_error("Event Scheduler: "
+                    "[%s].[%s.%s] execution failed, "
+                    "user no longer has EVENT privilege.",
+                    definer.str, dbname.str, name.str);
+    goto end;
   }
 
-  event_restore_security_context(thd, &save_ctx);
-done:
-  thd->end_statement();
-  thd->cleanup_after_query();
-
-  DBUG_PRINT("info", ("EXECUTED %s.%s  ret: %d", dbname.str, name.str, ret));
-
-  DBUG_RETURN(ret);
-}
-
-
-/*
-  Compiles an event before it's execution. Compiles the anonymous
-  sp_head object held by the event
-
-  SYNOPSIS
-    Event_job_data::compile()
-      thd        thread context, used for memory allocation mostly
-      mem_root   if != NULL then this memory root is used for allocs
-                 instead of thd->mem_root
+  if (construct_sp_sql(thd, &sp_sql))
+    goto end;
 
-  RETURN VALUE
-    0                       success
-    EVEX_COMPILE_ERROR      error during compilation
-    EVEX_MICROSECOND_UNSUP  mysql.event was tampered
-*/
+  /*
+    Set up global thread attributes to reflect the properties of
+    this Event. We can simply reset these instead of usual
+    backup/restore employed in stored programs since we know that
+    this is a top level statement and the worker thread is
+    allocated exclusively to execute this event.
+  */
+  charset_connection= get_charset_by_csname("utf8",
+                                            MY_CS_PRIMARY, MYF(MY_WME));
+  thd->variables.character_set_client= charset_connection;
+  thd->variables.character_set_results= charset_connection;
+  thd->variables.collation_connection= charset_connection;
+  thd->update_charset();
 
-int
-Event_job_data::compile(THD *thd, MEM_ROOT *mem_root)
-{
-  int ret= 0;
-  MEM_ROOT *tmp_mem_root= 0;
-  LEX *old_lex= thd->lex, lex;
-  char *old_db;
-  int old_db_length;
-  char *old_query;
-  uint old_query_len;
-  ulong old_sql_mode= thd->variables.sql_mode;
-  char create_buf[15 * STRING_BUFFER_USUAL_SIZE];
-  String show_create(create_buf, sizeof(create_buf), system_charset_info);
-  CHARSET_INFO *old_character_set_client,
-               *old_collation_connection,
-               *old_character_set_results;
-  Security_context save_ctx;
+  thd->variables.sql_mode= sql_mode;
+  thd->variables.time_zone= time_zone;
 
-  DBUG_ENTER("Event_job_data::compile");
+  thd->query= sp_sql.c_ptr_safe();
+  thd->query_length= sp_sql.length();
 
-  show_create.length(0);
+  lex_start(thd, thd->query, thd->query_length);
 
-  switch (get_fake_create_event(&show_create)) {
-  case EVEX_MICROSECOND_UNSUP:
-    DBUG_RETURN(EVEX_MICROSECOND_UNSUP);
-  case 0:
-    break;
-  default:
-    DBUG_ASSERT(0);
+  if (MYSQLparse(thd) || thd->is_fatal_error)
+  {
+    sql_print_error("Event Scheduler: "
+                    "%serror during compilation of %s.%s",
+                    thd->is_fatal_error ? "fatal " : "",
+                    (const char *) dbname.str, (const char *) name.str);
+    goto end;
   }
 
-  old_character_set_client= thd->variables.character_set_client;
-  old_character_set_results= thd->variables.character_set_results;
-  old_collation_connection= thd->variables.collation_connection;
-
-  thd->variables.character_set_client=
-    thd->variables.character_set_results=
-      thd->variables.collation_connection=
-           get_charset_by_csname("utf8", MY_CS_PRIMARY, MYF(MY_WME));
+  {
+    sp_head *sphead= thd->lex->sphead;
 
-  thd->update_charset();
+    DBUG_ASSERT(sphead);
 
-  DBUG_PRINT("info",("old_sql_mode: %lu  new_sql_mode: %lu",old_sql_mode, sql_mode));
-  thd->variables.sql_mode= this->sql_mode;
-  /* Change the memory root for the execution time */
-  if (mem_root)
-  {
-    tmp_mem_root= thd->mem_root;
-    thd->mem_root= mem_root;
-  }
-  old_query_len= thd->query_length;
-  old_query= thd->query;
-  old_db= thd->db;
-  old_db_length= thd->db_length;
-  thd->db= dbname.str;
-  thd->db_length= dbname.length;
-
-  thd->query= show_create.c_ptr_safe();
-  thd->query_length= show_create.length();
-  DBUG_PRINT("info", ("query: %s",thd->query));
-
-  event_change_security_context(thd, definer_user, definer_host, dbname,
-                                &save_ctx);
-  thd->lex= &lex;
-  mysql_init_query(thd, thd->query, thd->query_length);
-  if (MYSQLparse((void *)thd) || thd->is_fatal_error)
-  {
-    DBUG_PRINT("error", ("error during compile or thd->is_fatal_error: %d",
-                          thd->is_fatal_error));
-    lex.unit.cleanup();
+    if (thd->enable_slow_log)
+      sphead->m_flags|= sp_head::LOG_SLOW_STATEMENTS;
+    sphead->m_flags|= sp_head::LOG_GENERAL_LOG;
 
-    sql_print_error("Event Scheduler: "
-                    "%serror during compilation of %s.%s",
-                    thd->is_fatal_error ? "fatal " : "",
-                    dbname.str, name.str);
+    sphead->set_info(0, 0, &thd->lex->sp_chistics, sql_mode);
+    sphead->optimize();
 
-    ret= EVEX_COMPILE_ERROR;
-    goto done;
+    ret= sphead->execute_procedure(thd, &empty_item_list);
+    /*
+      There is no pre-locking and therefore there should be no
+      tables open and locked left after execute_procedure.
+    */
   }
-  DBUG_PRINT("note", ("success compiling %s.%s", dbname.str, name.str));
-
-  sphead= lex.sphead;
-
-  sphead->set_definer(definer.str, definer.length);
-  sphead->set_info(0, 0, &lex.sp_chistics, sql_mode);
-  sphead->optimize();
-  ret= 0;
-done:
 
-  lex_end(&lex);
-  event_restore_security_context(thd, &save_ctx);
-  DBUG_PRINT("note", ("return old data on its place. set back NAMES"));
-
-  thd->lex= old_lex;
-  thd->query= old_query;
-  thd->query_length= old_query_len;
-  thd->db= old_db;
-
-  thd->variables.sql_mode= old_sql_mode;
-  thd->variables.character_set_client= old_character_set_client;
-  thd->variables.character_set_results= old_character_set_results;
-  thd->variables.collation_connection= old_collation_connection;
-  thd->update_charset();
+end:
+  if (drop && !thd->is_fatal_error)
+  {
+    sql_print_information("Event Scheduler: Dropping %s.%s",
+                          (const char *) dbname.str, (const char *) name.str);
+    /*
+      We must do it here since here we're under the right authentication
+      ID of the event definer.
+    */
+    if (Events::drop_event(thd, dbname, name, FALSE))
+      ret= 1;
+  }
+  if (thd->lex->sphead)                        /* NULL only if a parse error */
+  {
+    delete thd->lex->sphead;
+    thd->lex->sphead= NULL;
+  }
+#ifndef NO_EMBEDDED_ACCESS_CHECKS
+  if (save_sctx)
+    event_sctx.restore_security_context(thd, save_sctx);
+#endif
+  lex_end(thd->lex);
+  thd->lex->unit.cleanup();
+  thd->end_statement();
+  thd->cleanup_after_query();
 
-  /* Change the memory root for the execution time. */
-  if (mem_root)
-    thd->mem_root= tmp_mem_root;
+  DBUG_PRINT("info", ("EXECUTED %s.%s  ret: %d", dbname.str, name.str, ret));
 
   DBUG_RETURN(ret);
 }
@@ -2041,65 +1984,4 @@
 {
   return !sortcmp_lex_string(name, b->name, system_charset_info) &&
          !sortcmp_lex_string(db, b->dbname, system_charset_info);
-}
-
-
-/*
-  Switches the security context.
-
-  SYNOPSIS
-    event_change_security_context()
-      thd     Thread
-      user    The user
-      host    The host of the user
-      db      The schema for which the security_ctx will be loaded
-      backup  Where to store the old context
-
-  RETURN VALUE
-    FALSE  OK
-    TRUE   Error (generates error too)
-*/
-
-static bool
-event_change_security_context(THD *thd, LEX_STRING user, LEX_STRING host,
-                              LEX_STRING db, Security_context *backup)
-{
-  DBUG_ENTER("event_change_security_context");
-  DBUG_PRINT("info",("%s@%s@%s", user.str, host.str, db.str));
-#ifndef NO_EMBEDDED_ACCESS_CHECKS
-
-  *backup= thd->main_security_ctx;
-  if (acl_getroot_no_password(&thd->main_security_ctx, user.str, host.str,
-                              host.str, db.str))
-  {
-    my_error(ER_NO_SUCH_USER, MYF(0), user.str, host.str);
-    DBUG_RETURN(TRUE);
-  }
-  thd->security_ctx= &thd->main_security_ctx;
-#endif
-  DBUG_RETURN(FALSE);
-}
-
-
-/*
-  Restores the security context.
-
-  SYNOPSIS
-    event_restore_security_context()
-      thd     Thread
-      backup  Context to switch to
-*/
-
-static void
-event_restore_security_context(THD *thd, Security_context *backup)
-{
-  DBUG_ENTER("event_restore_security_context");
-#ifndef NO_EMBEDDED_ACCESS_CHECKS
-  if (backup)
-  {
-    thd->main_security_ctx= *backup;
-    thd->security_ctx= &thd->main_security_ctx;
-  }
-#endif
-  DBUG_VOID_RETURN;
 }

--- 1.22/sql/event_data_objects.h	2007-04-05 12:47:18 -04:00
+++ 1.23/sql/event_data_objects.h	2007-04-13 16:32:25 -04:00
@@ -17,7 +17,6 @@
 
 
 #define EVEX_GET_FIELD_FAILED   -2
-#define EVEX_COMPILE_ERROR      -3
 #define EVEX_BAD_PARAMS         -5
 #define EVEX_MICROSECOND_UNSUP  -6
 
@@ -169,8 +168,6 @@
 class Event_job_data : public Event_basic
 {
 public:
-  sp_head *sphead;
-
   LEX_STRING body;
   LEX_STRING definer_user;
   LEX_STRING definer_host;
@@ -178,19 +175,15 @@
   ulong sql_mode;
 
   Event_job_data();
-  virtual ~Event_job_data();
 
   virtual int
   load_from_row(THD *thd, TABLE *table);
 
-  int
+  bool
   execute(THD *thd, bool drop);
-
-  int
-  compile(THD *thd, MEM_ROOT *mem_root);
 private:
-  int
-  get_fake_create_event(String *buf);
+  bool
+  construct_sp_sql(THD *thd, String *sp_sql);
 
   Event_job_data(const Event_job_data &);       /* Prevent use of these */
   void operator=(Event_job_data &);

--- 1.87/sql/sql_trigger.cc	2007-03-27 13:09:52 -04:00
+++ 1.88/sql/sql_trigger.cc	2007-04-13 16:32:26 -04:00
@@ -1543,9 +1543,15 @@
       old_field= trigger_table->field;
     }
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
+    Security_context *sctx= &sp_trigger->m_security_ctx;
     Security_context *save_ctx;
 
-    if (sp_change_security_context(thd, sp_trigger, &save_ctx))
+
+    if (sctx->change_security_context(thd,
+                                      &sp_trigger->m_definer_user,
+                                      &sp_trigger->m_definer_host,
+                                      &sp_trigger->m_db,
+                                      &save_ctx))
       return TRUE;
 
     /*
@@ -1570,7 +1576,7 @@
                thd->security_ctx->priv_user, thd->security_ctx->host_or_ip,
                trigger_table->s->table_name.str);
 
-      sp_restore_security_context(thd, save_ctx);
+      sctx->restore_security_context(thd, save_ctx);
       return TRUE;
     }
 #endif // NO_EMBEDDED_ACCESS_CHECKS
@@ -1582,7 +1588,7 @@
     thd->restore_sub_statement_state(&statement_state);
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-    sp_restore_security_context(thd, save_ctx);
+    sctx->restore_security_context(thd, save_ctx);
 #endif // NO_EMBEDDED_ACCESS_CHECKS
   }
 

--- 1.265/sql/sp_head.cc	2007-04-06 13:18:21 -04:00
+++ 1.266/sql/sp_head.cc	2007-04-13 16:32:26 -04:00
@@ -1232,7 +1232,11 @@
                          Security_context **save_ctx)
 {
   *save_ctx= 0;
-  if (sp_change_security_context(thd, sp, save_ctx))
+  if (sp->m_chistics->suid != SP_IS_NOT_SUID &&
+      sp->m_security_ctx.change_security_context(thd, &sp->m_definer_user,
+                                                 &sp->m_definer_host,
+                                                 &sp->m_db,
+                                                 save_ctx))
     return TRUE;
 
   /*
@@ -1249,7 +1253,7 @@
       check_routine_access(thd, EXECUTE_ACL,
                            sp->m_db.str, sp->m_name.str, is_proc, FALSE))
   {
-    sp_restore_security_context(thd, *save_ctx);
+    sp->m_security_ctx.restore_security_context(thd, *save_ctx);
     *save_ctx= 0;
     return TRUE;
   }
@@ -1560,7 +1564,7 @@
   }
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-  sp_restore_security_context(thd, save_security_ctx);
+  m_security_ctx.restore_security_context(thd, save_security_ctx);
 #endif
 
 err_with_cleanup:
@@ -1778,7 +1782,7 @@
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   if (save_security_ctx)
-    sp_restore_security_context(thd, save_security_ctx);
+    m_security_ctx.restore_security_context(thd, save_security_ctx);
 #endif
 
   if (!save_spcont)
@@ -3418,44 +3422,6 @@
 
 /* ------------------------------------------------------------------ */
 
-/*
-  Security context swapping
-*/
-
-#ifndef NO_EMBEDDED_ACCESS_CHECKS
-bool
-sp_change_security_context(THD *thd, sp_head *sp, Security_context **backup)
-{
-  *backup= 0;
-  if (sp->m_chistics->suid != SP_IS_NOT_SUID &&
-      (strcmp(sp->m_definer_user.str,
-              thd->security_ctx->priv_user) ||
-       my_strcasecmp(system_charset_info, sp->m_definer_host.str,
-                     thd->security_ctx->priv_host)))
-  {
-    if (acl_getroot_no_password(&sp->m_security_ctx, sp->m_definer_user.str,
-                                sp->m_definer_host.str,
-                                sp->m_definer_host.str,
-                                sp->m_db.str))
-    {
-      my_error(ER_NO_SUCH_USER, MYF(0), sp->m_definer_user.str,
-               sp->m_definer_host.str);
-      return TRUE;
-    }
-    *backup= thd->security_ctx;
-    thd->security_ctx= &sp->m_security_ctx;
-  }
-  return FALSE;
-}
-
-void
-sp_restore_security_context(THD *thd, Security_context *backup)
-{
-  if (backup)
-    thd->security_ctx= backup;
-}
-
-#endif /* NO_EMBEDDED_ACCESS_CHECKS */
 
 /*
   Structure that represent all instances of one table
Thread
bk commit into 5.1 tree (kostja:1.2563) BUG#27733konstantin13 Apr