MDEV-5816: Stored programs: validation of stored program statements

Re-designed a way by that Item_trigger_field objects are arranged in memory.

Item_trigger_field objects created on parsing a trigger's statement
is now stored in a per statement list. All lists of Item_trigger_field
objects created on parsing the whole trigger's body are organized
in the structure "list of lists". So, use binary cycle to iterate every
Item_trigger_field object created on parsing a trigger body.

To organize the data structure 'list of lists' the new data member
  Item_trigger_field::next_trig_field_list
is introduced that links lists in this hierarchy structure.

This re-design is performed in order to avoid refences to already
deleted items on re-compilation of failed trigger's statememt.
Referencing to already deleted items could take place on re-parsing
a trigger's statement since every Item created for a statement
being re-parsed is deleted before the statement be re-parsed,
but deleted items are still referenced from sp_head. So, to avoid
access to dangling references a per statement list of Item_trigger_field
objects are cleared right after the current SP statement be cleaned up
and before re-parsing be started.

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<Item_trigger_field> *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)
   {

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<Item_trigger_field> *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);
 }
 

sql/sp_head.h

@@ -980,13 +980,13 @@ protected:
 
 public:
   /*
-    List of all items (Item_trigger_field objects) representing fields in
+    List of lists of Item_trigger_field objects representing all fields in
-    old/new version of row in trigger. We use this list for checking whenever
+    old/new version of row in trigger. We use this list of lists for checking
-    all such fields are valid at trigger creation time and for binding these
+    whenever all such fields are valid at trigger creation time and for binding
-    fields to TABLE object at table open (altough for latter pointer to table
+    these fields to TABLE object at table open (although for latter pointer
-    being opened is probably enough).
+    to table being opened is probably enough).
   */
-  SQL_I_List<Item_trigger_field> m_trg_table_fields;
+  SQL_I_List<SQL_I_List<Item_trigger_field> > 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<Item_trigger_field> m_cur_instr_trig_field_items;
 }; // class sp_head : public Sql_alloc
 
 

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<Item_trigger_field> *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<Item_trigger_field> *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;

sql/sp_instr.h

@@ -198,6 +198,10 @@ public:
 
   virtual PSI_statement_info* get_psi_info() = 0;
 
+  virtual SQL_I_List<Item_trigger_field>* 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<Item_trigger_field>* 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<Item_trigger_field> 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<Item_trigger_field> *next_trig_items_list);
 };
 
 

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;
 }

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 <typename FN>
+static
+bool iterate_trigger_fields_and_run_func(
+  SQL_I_List<SQL_I_List<Item_trigger_field> > &trg_table_fields,
+  FN fn
+  )
+{
+  for (SQL_I_List<Item_trigger_field>
+         *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;
+  if (iterate_trigger_fields_and_run_func(
-       trg_field; trg_field= trg_field->next_trg_field)
+        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.
+            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);
+          */
+          trg_field->setup_field(thd, table, nullptr);
 
-    if (trg_field->fix_fields_if_needed(thd, (Item **)0))
+          return trg_field->fix_fields_if_needed(thd, (Item **)0);
-      DBUG_RETURN(true);
+        }
-  }
+     ))
+    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;
+        (void)iterate_trigger_fields_and_run_func(
-             trg_field= trg_field->next_trg_field)
+          sp->m_trg_table_fields,
-        {
+          [thd, table, trigger] (Item_trigger_field* trg_field)
-          trg_field->setup_field(thd, table,
+          {
-                                 &trigger->subject_table_grants);
+            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;
+        Skip a trigger that was parsed with an error.
-           trg_field= trg_field->next_trg_field)
+      */
-      {
+      if (trigger->body == nullptr)
-        /* We cannot mark fields which does not present in table. */
+        continue;
-        if (trg_field->field_idx != NO_CACHED_FIELD_INDEX)
+
+      (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));
+          /* We cannot mark fields which does not present in table. */
-          if (trg_field->get_settable_routine_parameter())
+          if (trg_field->field_idx != NO_CACHED_FIELD_INDEX)
-            bitmap_set_bit(trigger_table->write_set, trg_field->field_idx);
+          {
-          trigger_table->mark_column_with_deps(
+            DBUG_PRINT("info", ("marking field: %u", (uint) trg_field->field_idx));
-                                  trigger_table->field[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();

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;