WL#1218 "Triggers"

After review and after merge fixes.

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