- last bits of unneeded error checking in sql_parse.cc thrown away

- fix a bug introduced with last commit ALTER EVENT a RENAME TO b; failed
- misc

sql/event.cc

@@ -192,23 +192,23 @@ TABLE *evex_open_event_table(THD *thd, enum thr_lock_type lock_type)
   Find row in open mysql.event table representing event
 
   SYNOPSIS
-    evex_db_find_routine_aux()
+    evex_db_find_event_aux()
       thd    Thread context
       dbname Name of event's database
       rname  Name of the event inside the db  
       table  TABLE object for open mysql.event table.
 
   RETURN VALUE
-    0                - Routine found
-    SP_KEY_NOT_FOUND- No routine with given name
+    0                  - Routine found
+    EVEX_KEY_NOT_FOUND - No routine with given name
 */
 
 int
-evex_db_find_routine_aux(THD *thd, const LEX_STRING dbname,
+evex_db_find_event_aux(THD *thd, const LEX_STRING dbname,
                        const LEX_STRING ev_name, TABLE *table)
 {
-  byte key[MAX_KEY_LENGTH];	// db, name, optional key length type
-  DBUG_ENTER("evex_db_find_routine_aux");
+  byte key[MAX_KEY_LENGTH];
+  DBUG_ENTER("evex_db_find_event_aux");
   DBUG_PRINT("enter", ("name: %.*s", ev_name.length, ev_name.str));
 
   /*
@@ -218,7 +218,8 @@ evex_db_find_routine_aux(THD *thd, const LEX_STRING dbname,
     'db' and 'name' and the first key is the primary key over the
     same fields.
   */
-  if (ev_name.length > table->field[1]->field_length)
+  if (dbname.length > table->field[EVEX_FIELD_DB]->field_length ||
+      ev_name.length > table->field[EVEX_FIELD_NAME]->field_length)
     DBUG_RETURN(EVEX_KEY_NOT_FOUND);
 
   table->field[0]->store(dbname.str, dbname.length, &my_charset_bin);
@@ -337,6 +338,9 @@ evex_fill_row(THD *thd, TABLE *table, event_timed *et, my_bool is_update)
        thd  THD
        et   event_timed object containing information for the event 
    
+     Return value
+                        0 - OK
+       EVEX_GENERAL_ERROR - Failure
    DESCRIPTION 
      Creates an event. Relies on evex_fill_row which is shared with 
      db_update_event. The name of the event is inside "et".
@@ -362,7 +366,7 @@ db_create_event(THD *thd, event_timed *et)
   }
 
   DBUG_PRINT("info", ("check existance of an event with the same name"));
-  if (!evex_db_find_routine_aux(thd, et->dbname, et->name, table))
+  if (!evex_db_find_event_aux(thd, et->dbname, et->name, table))
   {
     my_error(ER_EVENT_ALREADY_EXISTS, MYF(0), et->name.str);
     goto err;    
@@ -377,7 +381,6 @@ db_create_event(THD *thd, event_timed *et)
   
   restore_record(table, s->default_values); // Get default values for fields
 
-
   if (et->name.length > table->field[EVEX_FIELD_NAME]->field_length)
   {
     my_error(ER_TOO_LONG_IDENT, MYF(0), et->name.str);
@@ -392,28 +395,28 @@ db_create_event(THD *thd, event_timed *et)
   if (!(et->expression) && !(et->execute_at.year))
   {
     DBUG_PRINT("error", ("neither expression nor execute_at are set!"));
-    my_error(ER_EVENT_NEITHER_M_EXPR_NOR_M_AT, MYF(0));
-    
+    my_error(ER_EVENT_NEITHER_M_EXPR_NOR_M_AT, MYF(0));    
     goto err;
   }
 
   strxmov(definer, et->definer_user.str, "@", et->definer_host.str, NullS);
-  if (table->field[EVEX_FIELD_DEFINER]->
+  if ((ret=table->field[EVEX_FIELD_DEFINER]->
        store(definer, et->definer_user.length + 1 + et->definer_host.length,
-             system_charset_info))
+             system_charset_info)))
   {
-    my_error(ER_EVENT_STORE_FAILED, MYF(0), et->name.str);
+    my_error(ER_EVENT_STORE_FAILED, MYF(0), et->name.str, ret);
     goto err;
   }
 
   ((Field_timestamp *)table->field[EVEX_FIELD_CREATED])->set_time();
+
+  // evex_fill_row() calls my_error() in case of error so no need to handle it here
   if ((ret= evex_fill_row(thd, table, et, false)))
     goto err; 
 
-  ret= EVEX_OK;
   if (table->file->write_row(table->record[0]))
   {
-    my_error(ER_EVENT_STORE_FAILED, MYF(0), et->name.str);
+    my_error(ER_EVENT_STORE_FAILED, MYF(0), et->name.str, ret);
     goto err;
   }
   
@@ -438,7 +441,7 @@ db_create_event(THD *thd, event_timed *et)
 
 
 /*
-   Used to execute ALTER EVENT
+   Used to execute ALTER EVENT. Pendant to evex_update_event().
 
    SYNOPSIS
      db_update_event()
@@ -452,7 +455,7 @@ db_create_event(THD *thd, event_timed *et)
 */
 
 static int
-db_update_event(THD *thd, sp_name *new_name, event_timed *et)
+db_update_event(THD *thd, event_timed *et, sp_name *new_name)
 {
   TABLE *table;
   int ret= EVEX_OPEN_TABLE_FAILED;
@@ -467,13 +470,27 @@ db_update_event(THD *thd, sp_name *new_name, event_timed *et)
     my_error(ER_EVENT_OPEN_TABLE_FAILED, MYF(0));
     goto err;
   }
-
-  if (EVEX_KEY_NOT_FOUND == evex_db_find_routine_aux(thd, et->dbname, et->name,
+  
+  // first look whether we overwrite
+  if (new_name && !evex_db_find_event_aux(thd, new_name->m_db, new_name->m_name,
+                                            table))
+  {
+    my_error(ER_EVENT_ALREADY_EXISTS, MYF(0), new_name->m_name.str);
+    goto err;    
+  }
+  /*
+    ...and then whether there is such an event. don't exchange the blocks
+    because you will get error 120 from table handler because new_name will
+    overwrite the key and SE will tell us that it cannot find the already found
+    row (copied into record[1] later
+  */
+  if (EVEX_KEY_NOT_FOUND == evex_db_find_event_aux(thd, et->dbname, et->name,
                                                      table))
   {
     my_error(ER_EVENT_DOES_NOT_EXIST, MYF(0), et->name.str);
     goto err;    
   }
+
   
   store_record(table,record[1]);
   
@@ -494,7 +511,7 @@ db_update_event(THD *thd, sp_name *new_name, event_timed *et)
 
   if ((ret= table->file->update_row(table->record[1], table->record[0])))
   {
-    my_error(ER_EVENT_STORE_FAILED, MYF(0), et->name.str);
+    my_error(ER_EVENT_STORE_FAILED, MYF(0), et->name.str, ret);
     goto err;
   }
 
@@ -545,7 +562,7 @@ db_find_event(THD *thd, sp_name *name, event_timed **ett, TABLE *tbl)
     goto done;
   }
 
-  if ((ret= evex_db_find_routine_aux(thd, name->m_db, name->m_name, table)))
+  if ((ret= evex_db_find_event_aux(thd, name->m_db, name->m_name, table)))
   {
     my_error(ER_EVENT_DOES_NOT_EXIST, MYF(0), name->m_name.str);
     goto done;    
@@ -816,20 +833,20 @@ evex_create_event(THD *thd, event_timed *et, uint create_options)
 */
 
 int
-evex_update_event(THD *thd, event_timed *et, sp_name *name)
+evex_update_event(THD *thd, event_timed *et, sp_name *new_name)
 {
   int ret, i;
   bool need_second_pass= true;
-  sp_name *spn= 0;
 
   DBUG_ENTER("evex_update_event");
   DBUG_PRINT("enter", ("name: %*s", et->name.length, et->name.str));
 
   /*
     db_update_event() opens & closes the table to prevent
-    crash later in the code when loading and compiling the new definition
+    crash later in the code when loading and compiling the new definition.
+    Also on error conditions my_error() is called so no need to handle here
   */
-  if ((ret= db_update_event(thd, name, et)))
+  if ((ret= db_update_event(thd, et, new_name)))
     goto done;
 
   VOID(pthread_mutex_lock(&LOCK_evex_running));
@@ -839,22 +856,20 @@ evex_update_event(THD *thd, event_timed *et, sp_name *name)
   VOID(pthread_mutex_lock(&LOCK_event_arrays));
   evex_remove_from_cache(&et->dbname, &et->name, false);
   if (et->status == MYSQL_EVENT_ENABLED)
-    if (name)
-      ret= evex_load_and_compile_event(thd, name, false);
+  {
+    if (new_name)
+      ret= evex_load_and_compile_event(thd, new_name, false);
     else
     {
-      spn= new sp_name(et->dbname, et->name);
-      ret= evex_load_and_compile_event(thd, spn, false);
-      delete spn;
+      sp_name spn(et->dbname, et->name);
+      ret= evex_load_and_compile_event(thd, &spn, false);
     }
+    if (ret == EVEX_COMPILE_ERROR)
+      my_error(ER_EVENT_COMPILE_ERROR, MYF(0));
+  }
   VOID(pthread_mutex_unlock(&LOCK_event_arrays));
   VOID(pthread_mutex_unlock(&LOCK_evex_running));
 
-  /*
-    It is possible that 2 (or 1) pass(es) won't find the event in memory.
-    The reason is that DISABLED events are not cached.
-  */
-
 done:
   DBUG_RETURN(ret);
 }
@@ -885,7 +900,7 @@ evex_drop_event(THD *thd, event_timed *et, bool drop_if_exists)
     goto done;
   }
 
-  if (!(ret= evex_db_find_routine_aux(thd, et->dbname, et->name, table)))
+  if (!(ret= evex_db_find_event_aux(thd, et->dbname, et->name, table)))
   {
     if ((ret= table->file->delete_row(table->record[0])))
     { 	
@@ -893,15 +908,18 @@ evex_drop_event(THD *thd, event_timed *et, bool drop_if_exists)
       goto done;
     }
   }
-  else if (ret == SP_KEY_NOT_FOUND && drop_if_exists)
-  {
-     push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
+  else if (ret == EVEX_KEY_NOT_FOUND)
+  { 
+    if (drop_if_exists)
+    {
+      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
 		    ER_SP_DOES_NOT_EXIST, ER(ER_SP_DOES_NOT_EXIST),
-		    "EVENT", et->name.str);
-     ret= 0;
-     goto done;
-  } else
+		    "Event", et->name.str);
+      ret= 0;
+    } else
+      my_error(ER_EVENT_DOES_NOT_EXIST, MYF(0), et->name.str);
     goto done;
+  }
 
   VOID(pthread_mutex_lock(&LOCK_evex_running));
   if (evex_is_running)

sql/event.h

@@ -31,6 +31,7 @@ extern ulong opt_event_executor;
 #define EVEX_PARSE_ERROR        SP_PARSE_ERROR
 #define EVEX_INTERNAL_ERROR     SP_INTERNAL_ERROR
 #define EVEX_NO_DB_ERROR        SP_NO_DB_ERROR
+#define EVEX_COMPILE_ERROR     -19
 #define EVEX_GENERAL_ERROR     -20
 #define EVEX_BAD_IDENTIFIER     SP_BAD_IDENTIFIER
 #define EVEX_BODY_TOO_LONG      SP_BODY_TOO_LONG

sql/event_priv.h

@@ -55,7 +55,7 @@ int
 my_time_compare(TIME *a, TIME *b);
 
 int
-evex_db_find_routine_aux(THD *thd, const LEX_STRING dbname,
+evex_db_find_event_aux(THD *thd, const LEX_STRING dbname,
                        const LEX_STRING rname, TABLE *table);
                        
 TABLE *

sql/event_timed.cc

@@ -740,7 +740,7 @@ event_timed::update_fields(THD *thd)
   if (!(table= evex_open_event_table(thd, TL_WRITE)))
     DBUG_RETURN(SP_OPEN_TABLE_FAILED);
 
-  if ((ret= evex_db_find_routine_aux(thd, dbname, name, table)))
+  if ((ret= evex_db_find_event_aux(thd, dbname, name, table)))
     goto done;
 
   store_record(table,record[1]);
@@ -893,6 +893,14 @@ event_timed::execute(THD *thd, MEM_ROOT *mem_root)
 }
 
 
+/*
+  Returns
+                     0 - Success
+    EVEX_COMPILE_ERROR - Error during compilation
+
+*/
+
+
 int
 event_timed::compile(THD *thd, MEM_ROOT *mem_root)
 {
@@ -936,7 +944,7 @@ event_timed::compile(THD *thd, MEM_ROOT *mem_root)
     // QQ: anything else ?
     lex_end(&lex);
     thd->lex= old_lex;
-    DBUG_RETURN(-1);
+    DBUG_RETURN(EVEX_COMPILE_ERROR);
   }
   
   sphead= lex.sphead;

sql/share/errmsg.txt

@@ -5724,7 +5724,7 @@ ER_PLUGIN_IS_NOT_LOADED
 ER_EVENT_ALREADY_EXISTS
         eng "Event %s already exists"
 ER_EVENT_STORE_FAILED
-        eng "Failed to create event %s"
+        eng "Failed to store event %s. Error code %d from storage engine."
 ER_EVENT_DOES_NOT_EXIST
         eng "Event %s does not exist"
 ER_EVENT_CANT_ALTER
@@ -5747,3 +5747,5 @@ ER_EVENT_CANNOT_LOAD_FROM_TABLE
         eng "Cannot load from mysql.event. Table probably corrupted"
 ER_EVENT_CANNOT_DELETE
         eng "Failed to delete the event from mysql.event"
+ER_EVENT_COMPILE_ERROR
+        eng "Error during compilation of event's body"

sql/sql_parse.cc

@@ -3674,96 +3674,40 @@ mysql_execute_command(THD *thd)
     break;
   }
   case SQLCOM_CREATE_EVENT:
-  {
-    DBUG_ASSERT(lex->et);
-    
-    if (! lex->et->dbname.str)
-    {
-      my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));
-      delete lex->et;
-      lex->et= 0;
-      goto error;
-    }
-    if (check_access(thd, EVENT_ACL, lex->et->dbname.str, 0, 0, 0,
-                     is_schema_db(lex->et->dbname.str)))
-      break;
-
-    if (!(res= evex_create_event(thd, lex->et, (uint) lex->create_info.options)))
-      send_ok(thd, 1);
-
-    /* lex->unit.cleanup() is called outside, no need to call it here */
-    delete lex->et;
-    delete lex->sphead;
-    lex->et= 0;
-    lex->sphead= 0;
-
-    break;
-  }
   case SQLCOM_ALTER_EVENT:
+  case SQLCOM_DROP_EVENT:
   {
     DBUG_ASSERT(lex->et);
-    if (! lex->et->dbname.str)
-    {
-      my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));
-      delete lex->et;
-      lex->et= 0;
-      goto error;
-    }
-    if (check_access(thd, EVENT_ACL, lex->et->dbname.str, 0, 0, 0,
-                     is_schema_db(lex->et->dbname.str)))
-      break;
+    do {
+      if (! lex->et->dbname.str)
+      {
+        my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));
+        res= true;
+        break;
+      }
+      if (check_access(thd, EVENT_ACL, lex->et->dbname.str, 0, 0, 0,
+                       is_schema_db(lex->et->dbname.str)))
+        break;
+      switch (lex->sql_command) {
+      case SQLCOM_CREATE_EVENT:
+        res= evex_create_event(thd, lex->et, (uint) lex->create_info.options);
+        break;
+      case SQLCOM_ALTER_EVENT:
+        res= evex_update_event(thd, lex->et, lex->spname);
+        break;
+      case SQLCOM_DROP_EVENT:
+        evex_drop_event(thd, lex->et, lex->drop_if_exists);
+      default:;
+      }
+      if (!res)
+        send_ok(thd, 1);
 
-    int result;
-    res= (result= evex_update_event(thd, lex->et, lex->spname));
-    switch (result) {
-    case EVEX_OK:
-      send_ok(thd, 1);
-      break;
-    case EVEX_KEY_NOT_FOUND:
-      my_error(ER_EVENT_DOES_NOT_EXIST, MYF(0), lex->et->name.str);
-      break;
-    default:
-      my_error(ER_EVENT_CANT_ALTER, MYF(0), lex->et->name.str);
-      break;
-    }
-    /* lex->unit.cleanup() is called outside, no need to call it here */
+      /* lex->unit.cleanup() is called outside, no need to call it here */
+    } while (0);  
     delete lex->et;
     delete lex->sphead;
     lex->et= 0;
     lex->sphead= 0;
-    
-    break;
-  }
-  case SQLCOM_DROP_EVENT:
-  {
-    DBUG_ASSERT(lex->et);
-    if (! lex->et->dbname.str)
-    {
-      my_message(ER_NO_DB_ERROR, ER(ER_NO_DB_ERROR), MYF(0));
-      delete lex->et;
-      lex->et= 0;
-      goto error;
-    }
-    if (check_access(thd, EVENT_ACL, lex->et->dbname.str, 0, 0, 0,
-                     is_schema_db(lex->et->dbname.str)))
-      break;
-
-    int result;
-    res= (result= evex_drop_event(thd, lex->et, lex->drop_if_exists));
-    switch (result) {
-    case EVEX_OK:
-      send_ok(thd, 1);
-      break;
-    case EVEX_KEY_NOT_FOUND:
-      my_error(ER_EVENT_DOES_NOT_EXIST, MYF(0), lex->et->name.str);
-      break;
-    default:
-      my_error(ER_EVENT_DROP_FAILED, MYF(0), lex->et->name.str);
-      break;
-    }
-    delete lex->et;
-    lex->et= 0;
-
     break;
   }
   case SQLCOM_SHOW_CREATE_EVENT: