diff --git a/mysql-test/r/rpl_trigger.result b/mysql-test/r/rpl_trigger.result index 3c740bf8e64..f8573eec75f 100644 --- a/mysql-test/r/rpl_trigger.result +++ b/mysql-test/r/rpl_trigger.result @@ -941,3 +941,30 @@ c ---> Cleaning up... DROP TABLE t1; DROP TABLE t2; +drop table if exists t1; +create table t1(a int, b varchar(50)); +drop trigger not_a_trigger; +ERROR HY000: Trigger does not exist +drop trigger if exists not_a_trigger; +Warnings: +Note 1360 Trigger does not exist +create trigger t1_bi before insert on t1 +for each row set NEW.b := "In trigger t1_bi"; +insert into t1 values (1, "a"); +drop trigger if exists t1_bi; +insert into t1 values (2, "b"); +drop trigger if exists t1_bi; +Warnings: +Note 1360 Trigger does not exist +insert into t1 values (3, "c"); +select * from t1; +a b +1 In trigger t1_bi +2 b +3 c +select * from t1; +a b +1 In trigger t1_bi +2 b +3 c +drop table t1; diff --git a/mysql-test/r/trigger.result b/mysql-test/r/trigger.result index 9d3ab9b1d7d..9f34f60eb1a 100644 --- a/mysql-test/r/trigger.result +++ b/mysql-test/r/trigger.result @@ -1256,4 +1256,26 @@ select @a; 20 drop table t1; drop function f1; +drop table if exists t1; +create table t1(a int, b varchar(50)); +drop trigger not_a_trigger; +ERROR HY000: Trigger does not exist +drop trigger if exists not_a_trigger; +Warnings: +Note 1360 Trigger does not exist +create trigger t1_bi before insert on t1 +for each row set NEW.b := "In trigger t1_bi"; +insert into t1 values (1, "a"); +drop trigger if exists t1_bi; +insert into t1 values (2, "b"); +drop trigger if exists t1_bi; +Warnings: +Note 1360 Trigger does not exist +insert into t1 values (3, "c"); +select * from t1; +a b +1 In trigger t1_bi +2 b +3 c +drop table t1; End of 5.0 tests diff --git a/mysql-test/t/rpl_trigger.test b/mysql-test/t/rpl_trigger.test index d6e9410b1d3..faba89e7a73 100644 --- a/mysql-test/t/rpl_trigger.test +++ b/mysql-test/t/rpl_trigger.test @@ -423,6 +423,43 @@ DROP TABLE t2; --sync_with_master --connection master +# +# BUG#23703: DROP TRIGGER needs an IF EXISTS +# + +connection master; + +--disable_warnings +drop table if exists t1; +--enable_warnings + +create table t1(a int, b varchar(50)); + +-- error ER_TRG_DOES_NOT_EXIST +drop trigger not_a_trigger; + +drop trigger if exists not_a_trigger; + +create trigger t1_bi before insert on t1 +for each row set NEW.b := "In trigger t1_bi"; + +insert into t1 values (1, "a"); +drop trigger if exists t1_bi; +insert into t1 values (2, "b"); +drop trigger if exists t1_bi; +insert into t1 values (3, "c"); + +select * from t1; + +save_master_pos; +connection slave; +sync_with_master; + +select * from t1; + +connection master; + +drop table t1; # # End of tests diff --git a/mysql-test/t/trigger.test b/mysql-test/t/trigger.test index 81334f9b8ef..a9395c12a63 100644 --- a/mysql-test/t/trigger.test +++ b/mysql-test/t/trigger.test @@ -1519,4 +1519,32 @@ connection default; drop table t1; drop function f1; +# +# Bug#23703: DROP TRIGGER needs an IF EXISTS +# + +--disable_warnings +drop table if exists t1; +--enable_warnings + +create table t1(a int, b varchar(50)); + +-- error ER_TRG_DOES_NOT_EXIST +drop trigger not_a_trigger; + +drop trigger if exists not_a_trigger; + +create trigger t1_bi before insert on t1 +for each row set NEW.b := "In trigger t1_bi"; + +insert into t1 values (1, "a"); +drop trigger if exists t1_bi; +insert into t1 values (2, "b"); +drop trigger if exists t1_bi; +insert into t1 values (3, "c"); + +select * from t1; + +drop table t1; + --echo End of 5.0 tests diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index 95734d31411..3552fc596f3 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -107,7 +107,9 @@ const LEX_STRING trg_event_type_names[]= }; -static TABLE_LIST *add_table_for_trigger(THD *thd, sp_name *trig); +static int +add_table_for_trigger(THD *thd, sp_name *trig, bool if_exists, + TABLE_LIST ** table); class Handle_old_incorrect_sql_modes_hook: public Unknown_key_hook { @@ -156,6 +158,13 @@ private: */ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) { + /* + FIXME: The code below takes too many different paths depending on the + 'create' flag, so that the justification for a single function + 'mysql_create_or_drop_trigger', compared to two separate functions + 'mysql_create_trigger' and 'mysql_drop_trigger' is not apparent. + This is a good candidate for a minor refactoring. + */ TABLE *table; bool result= TRUE; String stmt_query; @@ -181,10 +190,6 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) DBUG_RETURN(TRUE); } - if (!create && - !(tables= add_table_for_trigger(thd, thd->lex->spname))) - DBUG_RETURN(TRUE); - /* We don't allow creating triggers on tables in the 'mysql' schema */ @@ -194,9 +199,6 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) DBUG_RETURN(TRUE); } - /* We should have only one table in table list. */ - DBUG_ASSERT(tables->next_global == 0); - /* TODO: We should check if user has TRIGGER privilege for table here. Now we just require SUPER privilege for creating/dropping because @@ -211,7 +213,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) DROP for example) so we do the check for privileges. For now there is already a stronger test right above; but when this stronger test will be removed, the test below will hold. Because triggers have the same - nature as functions regarding binlogging: their body is implicitely + nature as functions regarding binlogging: their body is implicitly binlogged, so they share the same danger, so trust_function_creators applies to them too. */ @@ -222,24 +224,52 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) DBUG_RETURN(TRUE); } - /* We do not allow creation of triggers on temporary tables. */ - if (create && find_temporary_table(thd, tables->db, tables->table_name)) - { - my_error(ER_TRG_ON_VIEW_OR_TEMP_TABLE, MYF(0), tables->alias); - DBUG_RETURN(TRUE); - } - /* We don't want perform our operations while global read lock is held - so we have to wait until its end and then prevent it from occuring + so we have to wait until its end and then prevent it from occurring again until we are done. (Acquiring LOCK_open is not enough because - global read lock is held without helding LOCK_open). + global read lock is held without holding LOCK_open). */ if (wait_if_global_read_lock(thd, 0, 1)) DBUG_RETURN(TRUE); VOID(pthread_mutex_lock(&LOCK_open)); + if (!create) + { + bool if_exists= thd->lex->drop_if_exists; + + if (add_table_for_trigger(thd, thd->lex->spname, if_exists, & tables)) + goto end; + + if (!tables) + { + DBUG_ASSERT(if_exists); + /* + Since the trigger does not exist, there is no associated table, + and therefore : + - no TRIGGER privileges to check, + - no trigger to drop, + - no table to lock/modify, + so the drop statement is successful. + */ + result= FALSE; + /* Still, we need to log the query ... */ + stmt_query.append(thd->query, thd->query_length); + goto end; + } + } + + /* We should have only one table in table list. */ + DBUG_ASSERT(tables->next_global == 0); + + /* We do not allow creation of triggers on temporary tables. */ + if (create && find_temporary_table(thd, tables->db, tables->table_name)) + { + my_error(ER_TRG_ON_VIEW_OR_TEMP_TABLE, MYF(0), tables->alias); + goto end; + } + if (lock_table_names(thd, tables)) goto end; @@ -1145,13 +1175,17 @@ bool Table_triggers_list::get_trigger_info(THD *thd, trg_event_type event, mysql_table_for_trigger() thd - current thread context trig - identifier for trigger + if_exists - treat a not existing trigger as a warning if TRUE + table - pointer to TABLE_LIST object for the table trigger (output) RETURN VALUE - 0 - error - # - pointer to TABLE_LIST object for the table + 0 Success + 1 Error */ -static TABLE_LIST *add_table_for_trigger(THD *thd, sp_name *trig) +static int +add_table_for_trigger(THD *thd, sp_name *trig, bool if_exists, + TABLE_LIST **table) { LEX *lex= thd->lex; char path_buff[FN_REFLEN]; @@ -1162,6 +1196,7 @@ static TABLE_LIST *add_table_for_trigger(THD *thd, sp_name *trig) path_buff, &trigname.trigger_table); DBUG_ENTER("add_table_for_trigger"); + DBUG_ASSERT(table != NULL); strxnmov(path_buff, FN_REFLEN, mysql_data_home, "/", trig->m_db.str, "/", trig->m_name.str, trigname_file_ext, NullS); @@ -1170,30 +1205,45 @@ static TABLE_LIST *add_table_for_trigger(THD *thd, sp_name *trig) if (access(path_buff, F_OK)) { + if (if_exists) + { + push_warning_printf(thd, + MYSQL_ERROR::WARN_LEVEL_NOTE, + ER_TRG_DOES_NOT_EXIST, + ER(ER_TRG_DOES_NOT_EXIST)); + *table= NULL; + DBUG_RETURN(0); + } + my_error(ER_TRG_DOES_NOT_EXIST, MYF(0)); - DBUG_RETURN(0); + DBUG_RETURN(1); } if (!(parser= sql_parse_prepare(&path, thd->mem_root, 1))) - DBUG_RETURN(0); + DBUG_RETURN(1); if (!is_equal(&trigname_file_type, parser->type())) { my_error(ER_WRONG_OBJECT, MYF(0), trig->m_name.str, trigname_file_ext+1, "TRIGGERNAME"); - DBUG_RETURN(0); + DBUG_RETURN(1); } if (parser->parse((gptr)&trigname, thd->mem_root, trigname_file_parameters, 1, &trigger_table_hook)) - DBUG_RETURN(0); + DBUG_RETURN(1); /* We need to reset statement table list to be PS/SP friendly. */ lex->query_tables= 0; lex->query_tables_last= &lex->query_tables; - DBUG_RETURN(sp_add_to_query_tables(thd, lex, trig->m_db.str, - trigname.trigger_table.str, TL_IGNORE)); + *table= sp_add_to_query_tables(thd, lex, trig->m_db.str, + trigname.trigger_table.str, TL_IGNORE); + + if (! *table) + DBUG_RETURN(1); + + DBUG_RETURN(0); } diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 6f24a42c07c..21325123baa 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -6098,11 +6098,12 @@ drop: lex->sql_command= SQLCOM_DROP_VIEW; lex->drop_if_exists= $3; } - | DROP TRIGGER_SYM sp_name + | DROP TRIGGER_SYM if_exists sp_name { LEX *lex= Lex; lex->sql_command= SQLCOM_DROP_TRIGGER; - lex->spname= $3; + lex->drop_if_exists= $3; + lex->spname= $4; } ;