WL#1218 "Triggers"

After review and after merge fixes.


mysql-test/t/trigger.test:
  After merge fix. Updated error codes.
sql/sp_head.cc:
  After merge fix.
  To give some chances for functions/triggers we have to close tables during sp_instr_* 
  execution only if we have opened them before.
sql/sp_head.h:
  After merge fix. sp_instr constructor now takes one more argument.
sql/sql_trigger.cc:
  After merge and review fixes.
  Some variable renaming and optimizations.
sql/sql_yacc.yy:
  After merge fixes.
  sp_instr_* classes now require sp context as constructor parameter.
  Also we should be careful with adding table for which we are creating trigger to table 
  list. Some elements in trigger body can damage LEX::query_tables and so we should add this
  table to list only after parsing trigger body.

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;

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

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)
   {}

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<LEX_STRING> it(triggers_info->definitions_list);
+      List_iterator_fast<LEX_STRING> 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);

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