diff --git a/sql/item.h b/sql/item.h index bfc261e204c..60755abc617 100644 --- a/sql/item.h +++ b/sql/item.h @@ -6950,6 +6950,14 @@ private: public: /* Next in list of all Item_trigger_field's in trigger */ Item_trigger_field *next_trg_field; + + /** + Pointer to the next list of Item_trigger_field objects. This pointer + is used to organize an intrusive list of lists of Item_trigger_field + objects managed by sp_head. + */ + SQL_I_List *next_trig_field_list; + /* Pointer to Table_trigger_list object for table of this trigger */ Table_triggers_list *triggers; /* Is this item represents row from NEW or OLD row ? */ @@ -6984,7 +6992,8 @@ Item_trigger_field(THD *thd, Name_resolution_context *context_arg, const LEX_CSTRING &field_name_arg, privilege_t priv, const bool ro) :Item_field(thd, context_arg, field_name_arg), - table_grants(NULL), next_trg_field(NULL), triggers(NULL), + table_grants(nullptr), next_trg_field(nullptr), + next_trig_field_list(nullptr), triggers(nullptr), row_version(row_ver_arg), field_idx(NO_CACHED_FIELD_INDEX), read_only (ro), original_privilege(priv), want_privilege(priv) { diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 2919c2c7dfa..7cb82d8f41c 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -3052,6 +3052,39 @@ int sp_head::add_instr(sp_instr *instr) */ instr->mem_root= &main_mem_root; instr->m_lineno= m_thd->m_parser_state->m_lip.yylineno; + + /* + Check if SP is a trigger and there are Item_trigger_field objects + created on parsing the current SP instruction. + */ + if (m_handler->type() == SP_TYPE_TRIGGER && + m_cur_instr_trig_field_items.elements) + { + /* + Get a pointer to a list of Item_trigger_field objects owned by + the current SP instruction. This list is used for storing + Item_trigger_field objects that can be created on parsing + the SP instruction's statement. If the current SP instruction is not + an instance of the class sp_lex_instr or derived class then + the value nullptr is returned by the virtual method + get_instr_trig_field_list(). + */ + SQL_I_List *instr_trig_fld_list= + instr->get_instr_trig_field_list(); + + /* + If the current SP instruction can store a list of Item_trigger_field + objects created on its parsing then move these Item_trigger_field + objects to the SP instruction's list. + */ + if (instr_trig_fld_list) + { + m_cur_instr_trig_field_items.save_and_clear(instr_trig_fld_list); + m_trg_table_fields.link_in_list( + instr_trig_fld_list, + &instr_trig_fld_list->first->next_trig_field_list); + } + } return insert_dynamic(&m_instr, (uchar*)&instr); } diff --git a/sql/sp_head.h b/sql/sp_head.h index b621cfd4ad2..e225da6b9c1 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -980,13 +980,13 @@ protected: public: /* - List of all items (Item_trigger_field objects) representing fields in - old/new version of row in trigger. We use this list for checking whenever - all such fields are valid at trigger creation time and for binding these - fields to TABLE object at table open (altough for latter pointer to table - being opened is probably enough). + List of lists of Item_trigger_field objects representing all fields in + old/new version of row in trigger. We use this list of lists for checking + whenever all such fields are valid at trigger creation time and for binding + these fields to TABLE object at table open (although for latter pointer + to table being opened is probably enough). */ - SQL_I_List m_trg_table_fields; + SQL_I_List > m_trg_table_fields; /** The object of the Trigger class corresponding to this sp_head object. @@ -996,6 +996,12 @@ public: trigger's instruction. */ Trigger *m_trg= nullptr; + + /* + List of Item_trigger_field objects created on parsing + a current instruction of trigger's body + */ + SQL_I_List m_cur_instr_trig_field_items; }; // class sp_head : public Sql_alloc diff --git a/sql/sp_instr.cc b/sql/sp_instr.cc index 24e9fd5f1ec..b11f4bf2b53 100644 --- a/sql/sp_instr.cc +++ b/sql/sp_instr.cc @@ -546,7 +546,7 @@ void sp_lex_instr::get_query(String *sql_query) const } -void sp_lex_instr::cleanup_before_parsing() +void sp_lex_instr::cleanup_before_parsing(enum_sp_type sp_type) { Item *current= free_list; @@ -558,6 +558,14 @@ void sp_lex_instr::cleanup_before_parsing() } free_list= nullptr; + + if (sp_type == SP_TYPE_TRIGGER) + /* + Some of deleted items can be referenced from the list + m_cur_trigger_stmt_items. Clean up the list content to avoid + dangling references. + */ + m_cur_trigger_stmt_items.empty(); } @@ -568,17 +576,36 @@ void sp_lex_instr::cleanup_before_parsing() @param sp sp_head object of the trigger */ -static void setup_table_fields_for_trigger(THD *thd, sp_head *sp) +bool sp_lex_instr::setup_table_fields_for_trigger( + THD *thd, sp_head *sp, + SQL_I_List *next_trig_items_list) { + bool result= false; + DBUG_ASSERT(sp->m_trg); - for (Item_trigger_field *trg_field= sp->m_trg_table_fields.first; + for (Item_trigger_field *trg_field= sp->m_cur_instr_trig_field_items.first; trg_field; trg_field= trg_field->next_trg_field) { trg_field->setup_field(thd, sp->m_trg->base->get_subject_table(), &sp->m_trg->subject_table_grants); + result= trg_field->fix_fields_if_needed(thd, (Item **)0); } + + /* + Move the list of Item_trigger_field objects, that have just been + filled in on parsing the trigger's statement, into the instruction list + owned by SP instruction. + */ + if (sp->m_cur_instr_trig_field_items.elements) + { + sp->m_cur_instr_trig_field_items.save_and_clear( + &m_cur_trigger_stmt_items); + m_cur_trigger_stmt_items.first->next_trig_field_list= next_trig_items_list; + } + + return result; } LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp) @@ -599,7 +626,18 @@ LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp) return nullptr; } - cleanup_before_parsing(); + /* + Remember a pointer to the next list of Item_trigger_field objects. + The current list of Item_trigger_field objects is cleared up in the + method cleanup_before_parsing(). + */ + SQL_I_List *saved_ptr_to_next_trg_items_list= nullptr; + + if (m_cur_trigger_stmt_items.elements) + saved_ptr_to_next_trg_items_list= + m_cur_trigger_stmt_items.first->next_trig_field_list; + + cleanup_before_parsing(sp->m_handler->type()); Parser_state parser_state; @@ -668,7 +706,8 @@ LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp) parsing_failed= on_after_expr_parsing(thd); if (sp->m_handler->type() == SP_TYPE_TRIGGER) - setup_table_fields_for_trigger(thd, sp); + setup_table_fields_for_trigger(thd, sp, + saved_ptr_to_next_trg_items_list); /* Assign the list of items created on parsing to the current @@ -1101,7 +1140,7 @@ bool sp_instr_set_trigger_field::on_after_expr_parsing(THD *thd) if (!value || !trigger_field) return true; - thd->spcont->m_sp->m_trg_table_fields.link_in_list( + thd->spcont->m_sp->m_cur_instr_trig_field_items.link_in_list( trigger_field, &trigger_field->next_trg_field); return false; diff --git a/sql/sp_instr.h b/sql/sp_instr.h index 662dc4bce8b..b3c1ce84ce2 100644 --- a/sql/sp_instr.h +++ b/sql/sp_instr.h @@ -198,6 +198,10 @@ public: virtual PSI_statement_info* get_psi_info() = 0; + virtual SQL_I_List* get_instr_trig_field_list() + { + return nullptr; + } }; // class sp_instr : public Sql_alloc @@ -387,6 +391,11 @@ public: */ LEX *parse_expr(THD *thd, sp_head *sp); + SQL_I_List* get_instr_trig_field_list() override + { + return &m_cur_trigger_stmt_items; + } + protected: /** @return the expression query string. This string can't be passed directly @@ -420,10 +429,35 @@ protected: sp_lex_keeper m_lex_keeper; private: + /** + List of Item_trigger_field objects created on parsing of a SQL statement + corresponding to this SP-instruction. + */ + SQL_I_List m_cur_trigger_stmt_items; + /** Clean up items previously created on behalf of the current instruction. */ - void cleanup_before_parsing(); + void cleanup_before_parsing(enum_sp_type sp_type); + + + /** + Set up field object for every NEW/OLD item of the trigger and + move the list of Item_trigger_field objects, created on parsing the current + trigger's instruction, from sp_head to trigger's SP instruction object. + + @param thd current thread + @param sp sp_head object of the trigger + @param next_trig_items_list pointer to the next list of Item_trigger_field + objects that used as a link between lists + to support list of lists structure. + + @return false on success, true on failure + */ + + bool setup_table_fields_for_trigger( + THD *thd, sp_head *sp, + SQL_I_List *next_trig_items_list); }; diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 167d2ea7791..3ed16a2172a 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -242,7 +242,8 @@ bool LEX::set_trigger_new_row(const LEX_CSTRING *name, Item *val, Let us add this item to list of all Item_trigger_field objects in trigger. */ - sphead->m_trg_table_fields.link_in_list(trg_fld, &trg_fld->next_trg_field); + sphead->m_cur_instr_trig_field_items.link_in_list(trg_fld, + &trg_fld->next_trg_field); return sphead->add_instr(sp_fld); } @@ -7954,7 +7955,8 @@ Item *LEX::create_and_link_Item_trigger_field(THD *thd, in trigger. */ if (likely(trg_fld)) - sphead->m_trg_table_fields.link_in_list(trg_fld, &trg_fld->next_trg_field); + sphead->m_cur_instr_trig_field_items.link_in_list(trg_fld, + &trg_fld->next_trg_field); return trg_fld; } diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index 10e09dec18c..5304a535ad0 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -866,6 +866,41 @@ static void build_trig_stmt_query(THD *thd, TABLE_LIST *tables, } +/** + Visit every Item_trigger_field object associated with a trigger + and run the code supplied in the last argument, passing + the Item_trigger_fgield object being visited. + + @param trg_table_fields Item_trigger_field objects owned by a trigger + @param fn a function to invoke for every Item_trigger_field + object + + @return false on success, true on failure. +*/ + +template +static +bool iterate_trigger_fields_and_run_func( + SQL_I_List > &trg_table_fields, + FN fn + ) +{ + for (SQL_I_List + *trg_fld_lst= trg_table_fields.first; + trg_fld_lst; + trg_fld_lst= trg_fld_lst->first->next_trig_field_list) + { + for (Item_trigger_field *trg_field= trg_fld_lst->first; + trg_field; + trg_field= trg_field->next_trg_field) + { + if (fn(trg_field)) + return true; + } + } + return false; +} + /** Create trigger for table. @@ -902,7 +937,6 @@ bool Table_triggers_list::create_trigger(THD *thd, TABLE_LIST *tables, char trg_definer_holder[USER_HOST_BUFF_SIZE]; LEX_CSTRING backup_name= { backup_file_buff, 0 }; LEX_CSTRING file, trigname_file; - Item_trigger_field *trg_field; struct st_trigname trigname; String trigger_definition; Trigger *trigger= 0; @@ -939,18 +973,20 @@ bool Table_triggers_list::create_trigger(THD *thd, TABLE_LIST *tables, */ old_field= new_field= table->field; - for (trg_field= lex->sphead->m_trg_table_fields.first; - trg_field; trg_field= trg_field->next_trg_field) - { - /* - NOTE: now we do not check privileges at CREATE TRIGGER time. This will - be changed in the future. - */ - trg_field->setup_field(thd, table, NULL); + if (iterate_trigger_fields_and_run_func( + lex->sphead->m_trg_table_fields, + [thd, table] (Item_trigger_field* trg_field) + { + /* + NOTE: now we do not check privileges at CREATE TRIGGER time. + This will be changed in the future. + */ + trg_field->setup_field(thd, table, nullptr); - if (trg_field->fix_fields_if_needed(thd, (Item **)0)) - DBUG_RETURN(true); - } + return trg_field->fix_fields_if_needed(thd, (Item **)0); + } + )) + DBUG_RETURN(true); /* Ensure anchor trigger exists */ if (lex->trg_chistics.ordering_clause != TRG_ORDER_NONE) @@ -1797,12 +1833,6 @@ bool Table_triggers_list::check_n_load(THD *thd, const LEX_CSTRING *db, continue; } - /* - Gather all Item_trigger_field objects representing access to fields - in old/new versions of row in trigger into lists containing all such - objects for the trigger_list with same action and timing. - */ - trigger->trigger_fields= sp->m_trg_table_fields.first; /* Also let us bind these objects to Field objects in table being opened. @@ -1812,13 +1842,15 @@ bool Table_triggers_list::check_n_load(THD *thd, const LEX_CSTRING *db, SELECT)... Anyway some things can be checked only during trigger execution. */ - for (Item_trigger_field *trg_field= sp->m_trg_table_fields.first; - trg_field; - trg_field= trg_field->next_trg_field) - { - trg_field->setup_field(thd, table, - &trigger->subject_table_grants); - } + + (void)iterate_trigger_fields_and_run_func( + sp->m_trg_table_fields, + [thd, table, trigger] (Item_trigger_field* trg_field) + { + trg_field->setup_field(thd, table, &trigger->subject_table_grants); + return false; + } + ); sp->m_trg= trigger; lex_end(&lex); @@ -2579,7 +2611,6 @@ add_tables_and_routines_for_triggers(THD *thd, void Table_triggers_list::mark_fields_used(trg_event_type event) { int action_time; - Item_trigger_field *trg_field; DBUG_ENTER("Table_triggers_list::mark_fields_used"); for (action_time= 0; action_time < (int)TRG_ACTION_MAX; action_time++) @@ -2588,20 +2619,28 @@ void Table_triggers_list::mark_fields_used(trg_event_type event) trigger ; trigger= trigger->next) { - for (trg_field= trigger->trigger_fields; - trg_field; - trg_field= trg_field->next_trg_field) - { - /* We cannot mark fields which does not present in table. */ - if (trg_field->field_idx != NO_CACHED_FIELD_INDEX) + /* + Skip a trigger that was parsed with an error. + */ + if (trigger->body == nullptr) + continue; + + (void)iterate_trigger_fields_and_run_func( + trigger->body->m_trg_table_fields, + [this] (Item_trigger_field* trg_field) { - DBUG_PRINT("info", ("marking field: %u", (uint) trg_field->field_idx)); - if (trg_field->get_settable_routine_parameter()) - bitmap_set_bit(trigger_table->write_set, trg_field->field_idx); - trigger_table->mark_column_with_deps( - trigger_table->field[trg_field->field_idx]); + /* We cannot mark fields which does not present in table. */ + if (trg_field->field_idx != NO_CACHED_FIELD_INDEX) + { + DBUG_PRINT("info", ("marking field: %u", (uint) trg_field->field_idx)); + if (trg_field->get_settable_routine_parameter()) + bitmap_set_bit(trigger_table->write_set, trg_field->field_idx); + trigger_table->mark_column_with_deps( + trigger_table->field[trg_field->field_idx]); + } + return false; } - } + ); } } trigger_table->file->column_bitmaps_signal(); diff --git a/sql/sql_trigger.h b/sql/sql_trigger.h index d1edd94eb4f..493349afb9d 100644 --- a/sql/sql_trigger.h +++ b/sql/sql_trigger.h @@ -113,7 +113,7 @@ class Trigger :public Sql_alloc { public: Trigger(Table_triggers_list *base_arg, sp_head *code): - base(base_arg), body(code), next(0), trigger_fields(0), action_order(0) + base(base_arg), body(code), next(0), action_order(0) { bzero((char *)&subject_table_grants, sizeof(subject_table_grants)); } @@ -122,11 +122,6 @@ public: sp_head *body; Trigger *next; /* Next trigger of same type */ - /** - Heads of the lists linking items for all fields used in triggers - grouped by event and action_time. - */ - Item_trigger_field *trigger_fields; LEX_CSTRING name; LEX_CSTRING on_table_name; /* Raw table name */ LEX_CSTRING definition;