Bug#39953 Triggers are not working properly with multi table

updates

Attempt to execute trigger or stored function with multi-UPDATE
which used - but didn't update - a table that was also used by
the calling statement led to an error. Read-only reference to
tables used in the calling statement should be allowed.
 
This problem was caused by the fact that check for conflicting
use of tables in SP/triggers was performed in open_tables(),
and in case of multi-UPDATE we didn't know exact lock type at
this stage.

We solve the problem by moving this check to lock_tables(), so
it can be performed after exact lock types for tables used by
multi-UPDATE are determined.

mysql-test/r/trigger.result

@@ -1961,4 +1961,24 @@ select * from t2;
 s1
 drop table t1;
 drop temporary table t2;
+#------------------------------------------------------------------------
+# Bug#39953 Triggers are not working properly with multi table updates
+#------------------------------------------------------------------------
+DROP TABLE IF EXISTS t1;
+DROP TRIGGER IF EXISTS t_insert;
+DROP TABLE IF EXISTS t2;
+CREATE TABLE t1 (a int, date_insert timestamp, PRIMARY KEY (a));
+INSERT INTO t1 (a) VALUES (2),(5);
+CREATE TABLE t2 (a int, b int, PRIMARY KEY (a));
+CREATE TRIGGER t_insert AFTER INSERT ON t2 FOR EACH ROW BEGIN UPDATE t1,t2 SET
+date_insert=NOW() WHERE t1.a=t2.b AND t2.a=NEW.a; END |
+INSERT INTO t2 (a,b) VALUES (1,2);
+DROP TRIGGER t_insert;
+CREATE TRIGGER t_insert AFTER INSERT ON t2 FOR EACH ROW BEGIN UPDATE t1,t2 SET
+date_insert=NOW(),b=b+1 WHERE t1.a=t2.b AND t2.a=NEW.a; END |
+INSERT INTO t2 (a,b) VALUES (3,5);
+ERROR HY000: Can't update table 't2' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+DROP TABLE t1;
+DROP TRIGGER t_insert;
+DROP TABLE t2;
 End of 5.0 tests

mysql-test/t/trigger.test

@@ -2217,4 +2217,37 @@ select * from t1;
 select * from t2;
 drop table t1;
 drop temporary table t2;
+
+--echo #------------------------------------------------------------------------
+--echo # Bug#39953 Triggers are not working properly with multi table updates
+--echo #------------------------------------------------------------------------
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+DROP TRIGGER IF EXISTS t_insert;
+DROP TABLE IF EXISTS t2;
+--enable_warnings
+
+CREATE TABLE t1 (a int, date_insert timestamp, PRIMARY KEY (a));
+INSERT INTO t1 (a) VALUES (2),(5);
+CREATE TABLE t2 (a int, b int, PRIMARY KEY (a));
+DELIMITER |;
+CREATE TRIGGER t_insert AFTER INSERT ON t2 FOR EACH ROW BEGIN UPDATE t1,t2 SET
+date_insert=NOW() WHERE t1.a=t2.b AND t2.a=NEW.a; END |
+DELIMITER ;|
+INSERT INTO t2 (a,b) VALUES (1,2);
+
+DROP TRIGGER t_insert;
+
+DELIMITER |;
+CREATE TRIGGER t_insert AFTER INSERT ON t2 FOR EACH ROW BEGIN UPDATE t1,t2 SET
+date_insert=NOW(),b=b+1 WHERE t1.a=t2.b AND t2.a=NEW.a; END |
+DELIMITER ;|
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+INSERT INTO t2 (a,b) VALUES (3,5);
+
+DROP TABLE t1;
+DROP TRIGGER t_insert;
+DROP TABLE t2;
+
 --echo End of 5.0 tests

sql/sql_base.cc

@@ -1596,27 +1596,11 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
   {						// Using table locks
     TABLE *best_table= 0;
     int best_distance= INT_MIN;
-    bool check_if_used= thd->prelocked_mode &&
-                        ((int) table_list->lock_type >=
-                         (int) TL_WRITE_ALLOW_WRITE);
     for (table=thd->open_tables; table ; table=table->next)
     {
       if (table->s->key_length == key_length &&
           !memcmp(table->s->table_cache_key, key, key_length))
       {
-        if (check_if_used && table->query_id &&
-            table->query_id != thd->query_id)
-        {
-          /*
-            If we are in stored function or trigger we should ensure that
-            we won't change table that is already used by calling statement.
-            So if we are opening table for writing, we should check that it
-            is not already open by some calling stamement.
-          */
-          my_error(ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG, MYF(0),
-                   table->s->table_name);
-          DBUG_RETURN(0);
-        }
         if (!my_strcasecmp(system_charset_info, table->alias, alias) &&
             table->query_id != thd->query_id && /* skip tables already used */
             !(thd->prelocked_mode && table->query_id))
@@ -1640,13 +1624,13 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
           {
             best_distance= distance;
             best_table= table;
-            if (best_distance == 0 && !check_if_used)
+            if (best_distance == 0)
             {
               /*
-                If we have found perfect match and we don't need to check that
+                We have found a perfect match and can finish iterating
-                table is not used by one of calling statements (assuming that
+                through open tables list. Check for table use conflict
-                we are inside of function or trigger) we can finish iterating
+                between calling statement and SP/trigger is done in
-                through open tables list.
+                lock_tables().
               */
               break;
             }
@@ -2944,9 +2928,9 @@ static bool check_lock_and_start_stmt(THD *thd, TABLE *table,
     lock_type		Lock to use for open
 
   NOTE
-    This function don't do anything like SP/SF/views/triggers analysis done
+    This function doesn't do anything like SP/SF/views/triggers analysis done 
-    in open_tables(). It is intended for opening of only one concrete table.
+    in open_tables()/lock_tables(). It is intended for opening of only one
-    And used only in special contexts.
+    concrete table. And used only in special contexts.
 
   RETURN VALUES
     table		Opened table
@@ -3262,8 +3246,36 @@ int lock_tables(THD *thd, TABLE_LIST *tables, uint count, bool *need_reopen)
     TABLE_LIST *first_not_own= thd->lex->first_not_own_table();
     for (table= tables; table != first_not_own; table= table->next_global)
     {
-      if (!table->placeholder() &&
+      if (table->placeholder())
-	  check_lock_and_start_stmt(thd, table->table, table->lock_type))
+        continue;
+
+      /*
+        In a stored function or trigger we should ensure that we won't change
+        a table that is already used by the calling statement.
+      */
+      if (thd->prelocked_mode &&
+          table->lock_type >= TL_WRITE_ALLOW_WRITE)
+      {
+        for (TABLE* opentab= thd->open_tables; opentab; opentab= opentab->next)
+        {
+          /* 
+            issue an error if the tables are the same (by key comparison),
+            but query_id isn't
+          */
+          if (opentab->query_id && 
+              table->table->query_id != opentab->query_id &&
+              table->table->s->key_length == opentab->s->key_length &&
+              !memcmp(table->table->s->table_cache_key,
+                      opentab->s->table_cache_key, opentab->s->key_length))
+          {
+            my_error(ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG, MYF(0),
+                     table->table->s->table_name);
+            DBUG_RETURN(-1);
+          }
+        }
+      }
+
+      if (check_lock_and_start_stmt(thd, table->table, table->lock_type))
       {
 	ha_rollback_stmt(thd);
 	DBUG_RETURN(-1);