diff --git a/mysql-test/t/trigger.test b/mysql-test/t/trigger.test index f7b9ebc8d2c..edbd2a92c3c 100644 --- a/mysql-test/t/trigger.test +++ b/mysql-test/t/trigger.test @@ -151,15 +151,15 @@ drop table t1; # create table t1 (i int); ---error 1358 +--error 1362 create trigger trg before insert on t1 for each row set @a:= old.i; ---error 1358 +--error 1362 create trigger trg before delete on t1 for each row set @a:= new.i; ---error 1357 +--error 1361 create trigger trg before update on t1 for each row set old.i:=1; ---error 1358 +--error 1362 create trigger trg before delete on t1 for each row set new.i:=1; ---error 1357 +--error 1361 create trigger trg after update on t1 for each row set new.i:=1; # TODO: We should also test wrong field names here, we don't do it now # because proper error handling is not in place yet. @@ -173,23 +173,23 @@ create trigger trg after update on t1 for each row set new.i:=1; create trigger trg before insert on t2 for each row set @a:=1; create trigger trg before insert on t1 for each row set @a:=1; ---error 1354 +--error 1358 create trigger trg after insert on t1 for each row set @a:=1; ---error 1354 +--error 1358 create trigger trg2 before insert on t1 for each row set @a:=1; drop trigger t1.trg; ---error 1355 +--error 1359 drop trigger t1.trg; create view v1 as select * from t1; ---error 1356 +--error 1360 create trigger trg before insert on v1 for each row set @a:=1; drop view v1; drop table t1; create temporary table t1 (i int); ---error 1356 +--error 1360 create trigger trg before insert on t1 for each row set @a:=1; drop table t1; diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 3ff5f06103f..176a612d33d 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -1230,7 +1230,7 @@ sp_instr_set::execute(THD *thd, uint *nextp) thd->spcont->set_item(m_offset, it); } *nextp = m_ip+1; - if (thd->lock || thd->open_tables || thd->derived_tables) + if (tables && (thd->lock || thd->open_tables || thd->derived_tables)) close_thread_tables(thd); DBUG_RETURN(res); } @@ -1387,7 +1387,7 @@ sp_instr_jump_if::execute(THD *thd, uint *nextp) else *nextp = m_ip+1; } - if (thd->lock || thd->open_tables || thd->derived_tables) + if (tables && (thd->lock || thd->open_tables || thd->derived_tables)) close_thread_tables(thd); DBUG_RETURN(res); } @@ -1444,7 +1444,7 @@ sp_instr_jump_if_not::execute(THD *thd, uint *nextp) else *nextp = m_ip+1; } - if (thd->lock || thd->open_tables || thd->derived_tables) + if (tables && (thd->lock || thd->open_tables || thd->derived_tables)) close_thread_tables(thd); DBUG_RETURN(res); } diff --git a/sql/sp_head.h b/sql/sp_head.h index a253d9edcbe..f4ed3760b94 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -392,8 +392,8 @@ class sp_instr_set_user_var : public sp_instr public: - sp_instr_set_user_var(uint ip, LEX_STRING var, Item *val) - : sp_instr(ip), m_set_var_item(var, val) + sp_instr_set_user_var(uint ip, sp_pcontext *ctx, LEX_STRING var, Item *val) + : sp_instr(ip, ctx), m_set_var_item(var, val) {} virtual ~sp_instr_set_user_var() @@ -419,8 +419,9 @@ class sp_instr_set_trigger_field : public sp_instr public: - sp_instr_set_trigger_field(uint ip, LEX_STRING field_name, Item *val) - : sp_instr(ip), + sp_instr_set_trigger_field(uint ip, sp_pcontext *ctx, + LEX_STRING field_name, Item *val) + : sp_instr(ip, ctx), trigger_field(Item_trigger_field::NEW_ROW, field_name.str), value(val) {} diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index 5e08192d0c1..56294b9bc80 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -319,18 +319,18 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, if (!strncmp(triggers_file_type.str, parser->type()->str, parser->type()->length)) { - int i; - Table_triggers_list *triggers_info= + Field **fld, **old_fld; + Table_triggers_list *triggers= new (&table->mem_root) Table_triggers_list(); - if (!triggers_info) + if (!triggers) DBUG_RETURN(1); - if (parser->parse((gptr)triggers_info, &table->mem_root, + if (parser->parse((gptr)triggers, &table->mem_root, triggers_file_parameters, 1)) DBUG_RETURN(1); - table->triggers= triggers_info; + table->triggers= triggers; /* We have to prepare array of Field objects which will represent OLD.* @@ -338,27 +338,26 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, TODO: This could be avoided if there is no ON UPDATE trigger. */ - if (!(triggers_info->old_field= + if (!(triggers->old_field= (Field **)alloc_root(&table->mem_root, (table->fields + 1) * sizeof(Field*)))) DBUG_RETURN(1); - for (i= 0; i < table->fields; i++) + for (fld= table->field, old_fld= triggers->old_field; *fld; + fld++, old_fld++) { /* QQ: it is supposed that it is ok to use this function for field cloning... */ - if (!(triggers_info->old_field[i]= - table->field[i]->new_field(&table->mem_root, table))) + if (!(*old_fld= (*fld)->new_field(&table->mem_root, table))) DBUG_RETURN(1); - triggers_info->old_field[i]->move_field((my_ptrdiff_t) - (table->record[1] - - table->record[0])); + (*old_fld)->move_field((my_ptrdiff_t)(table->record[1] - + table->record[0])); } - triggers_info->old_field[i]= 0; + *old_fld= 0; - List_iterator_fast it(triggers_info->definitions_list); + List_iterator_fast it(triggers->definitions_list); LEX_STRING *trg_create_str, *trg_name_str; char *trg_name_buff; LEX *old_lex= thd->lex, lex; @@ -367,8 +366,8 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, while ((trg_create_str= it++)) { - lex_start(thd, (uchar*)trg_create_str->str, trg_create_str->length); - mysql_init_query(thd, true); + mysql_init_query(thd, (uchar*)trg_create_str->str, + trg_create_str->length, true); lex.trg_table= table; if (yyparse((void *)thd) || thd->is_fatal_error) { @@ -385,7 +384,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, goto err_with_lex_cleanup; } - triggers_info->bodies[lex.trg_chistics.event] + triggers->bodies[lex.trg_chistics.event] [lex.trg_chistics.action_time]= lex.sphead; lex.sphead= 0; @@ -404,7 +403,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db, old_global_mem_root= my_pthread_getspecific_ptr(MEM_ROOT*, THR_MALLOC); my_pthread_setspecific_ptr(THR_MALLOC, &table->mem_root); - if (triggers_info->names_list.push_back(trg_name_str)) + if (triggers->names_list.push_back(trg_name_str)) goto err_with_lex_cleanup; my_pthread_setspecific_ptr(THR_MALLOC, old_global_mem_root); diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 41432cee673..0f65aaa70da 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1212,15 +1212,6 @@ create: LEX *lex= Lex; sp_head *sp; - lex->name_and_length= $3; - - /* QQ: Could we loosen lock type in certain cases ? */ - if (!lex->select_lex.add_table_to_list(YYTHD, $7, - (LEX_STRING*) 0, - TL_OPTION_UPDATING, - TL_WRITE)) - YYABORT; - if (lex->sphead) { net_printf(YYTHD, ER_SP_NO_RECURSIVE_CREATE, "TRIGGER"); @@ -1256,6 +1247,23 @@ create: if (sp->m_old_cmq) YYTHD->client_capabilities |= CLIENT_MULTI_QUERIES; sp->restore_thd_mem_root(YYTHD); + + lex->name_and_length= $3; + + /* + We have to do it after parsing trigger body, because some of + sp_proc_stmt alternatives are not saving/restoring LEX, so + lex->query_tables can be wiped out. + + QQ: What are other consequences of this? + + QQ: Could we loosen lock type in certain cases ? + */ + if (!lex->select_lex.add_table_to_list(YYTHD, $7, + (LEX_STRING*) 0, + TL_OPTION_UPDATING, + TL_WRITE)) + YYABORT; } ; @@ -6922,10 +6930,18 @@ option_value: We have to use special instruction in functions and triggers because sp_instr_stmt will close all tables and thus ruin execution of statement invoking function or trigger. + + We also do not want to allow expression with subselects in + this case. */ + if (lex->query_tables) + { + send_error(YYTHD, ER_SP_SUBSELECT_NYI); + YYABORT; + } sp_instr_set_user_var *i= - new sp_instr_set_user_var(lex->sphead->instructions(), - $2, $4); + new sp_instr_set_user_var(lex->sphead->instructions(), + lex->spcont, $2, $4); lex->sphead->add_instr(i); } else @@ -6941,12 +6957,8 @@ option_value: /* We are in trigger and assigning value to field of new row */ Item *it; sp_instr_set_trigger_field *i; - if ($3 && $3->type() == Item::SUBSELECT_ITEM) - { /* - QQ For now, just disallow subselects as values - Unfortunately this doesn't helps in case when we have - subselect deeper in expression. - */ + if (lex->query_tables) + { send_error(YYTHD, ER_SP_SUBSELECT_NYI); YYABORT; } @@ -6958,7 +6970,7 @@ option_value: it= new Item_null(); } i= new sp_instr_set_trigger_field(lex->sphead->instructions(), - $1.base_name, it); + lex->spcont, $1.base_name, it); if (lex->trg_table && i->setup_field(YYTHD, lex->trg_table, lex->trg_chistics.event)) {