Bug#4118: multi-table UPDATE takes WRITE lock on read table

  Ensures that WRITE lock is not obtained on all tables referenced.


mysql-test/r/lock_multi.result:
  Bug#4118
    New test for multi-update locking
mysql-test/r/multi_update.result:
  Bug#4118
    Fix test
mysql-test/t/lock_multi.test:
  Bug#4118
    New test for multi-update locking
mysql-test/t/multi_update.test:
  Bug#4118
    Fix test
sql/sql_parse.cc:
  Bug#4118
    Split multi-update to its own case statement in sql_parse.cc
sql/sql_update.cc:
  Bug#4118
    Overview of locking checking:    
      1. Open and acquire READ lock
      2. Check to see which tables need WRITE lock
      3. Unlock tables and relock
sql/sql_yacc.yy:
  Bug#4118
    Split multi-update to its own case statement in sql_parse.cc

mysql-test/r/lock_multi.result

@@ -17,6 +17,18 @@ unlock tables;
 n
 1
 drop table t1;
+create table t1 (a int, b int);
+create table t2 (c int, d int);
+insert into t1 values(1,1);
+insert into t1 values(2,2);
+insert into t2 values(1,2);
+lock table t1 read;
+ update t1,t2 set c=a where b=d;
+select c from t2;
+c
+2
+drop table t1;
+drop table t2;
 create table t1 (a int);
 create table t2 (a int);
 lock table t1 write, t2 write;

mysql-test/r/multi_update.result

@@ -151,7 +151,6 @@ Table 't2' was locked with a READ lock and can't be updated
 UPDATE t1,t2 SET t1.d=t2.d,t2.d=30 WHERE t1.n=t2.n;
 Table 't2' was locked with a READ lock and can't be updated
 UPDATE t1,t2 SET t1.d=t2.d WHERE t1.n=t2.n;
-Table 't2' was locked with a READ lock and can't be updated
 unlock tables;
 LOCK TABLES t1 write, t2 write;
 UPDATE t1,t2 SET t1.d=t2.d WHERE t1.n=t2.n;

mysql-test/t/lock_multi.test

@@ -50,6 +50,30 @@ connection reader;
 reap;
 drop table t1;
 
+#
+# Test problem when using locks with multi-updates
+# It should not block when multi-update is reading on a read-locked table
+#
+
+connection locker;
+create table t1 (a int, b int);
+create table t2 (c int, d int);
+insert into t1 values(1,1);
+insert into t1 values(2,2);
+insert into t2 values(1,2);
+lock table t1 read;
+connection writer;
+--sleep 2
+send update t1,t2 set c=a where b=d;
+connection reader;
+--sleep 2
+select c from t2;
+connection writer;
+reap;
+connection locker;
+drop table t1;
+drop table t2;
+
 #
 # Test problem when using locks on many tables and droping a table that
 # is to-be-locked by another thread

mysql-test/t/multi_update.test

@@ -151,8 +151,6 @@ LOCK TABLES t1 write, t2 read;
 DELETE t1.*, t2.* FROM t1,t2 where t1.n=t2.n;
 --error 1099
 UPDATE t1,t2 SET t1.d=t2.d,t2.d=30 WHERE t1.n=t2.n;
-# The following should be fixed to not give an error
---error 1099
 UPDATE t1,t2 SET t1.d=t2.d WHERE t1.n=t2.n;
 unlock tables;
 LOCK TABLES t1 write, t2 write;

sql/sql_parse.cc

@@ -1927,8 +1927,6 @@ mysql_execute_command(void)
       send_error(&thd->net,ER_WRONG_VALUE_COUNT);
       DBUG_VOID_RETURN;
     }
-    if (select_lex->table_list.elements == 1)
-    {
     if (check_one_table_access(thd, UPDATE_ACL, tables, 0))
       goto error; /* purecov: inspected */
 
@@ -1940,8 +1938,15 @@ mysql_execute_command(void)
 		      (ORDER *) select_lex->order_list.first,
 		      select_lex->select_limit,
 		      lex->duplicates);
+    break;
+  case SQLCOM_MULTI_UPDATE:
+    if (check_db_used(thd,tables))
+      goto error;
+    if (select_lex->item_list.elements != lex->value_list.elements)
+    {
+      send_error(&thd->net,ER_WRONG_VALUE_COUNT);
+      DBUG_VOID_RETURN;
     }
-    else
     {
       const char *msg= 0;
       TABLE_LIST *table;

sql/sql_update.cc

@@ -401,9 +401,21 @@ int mysql_multi_update(THD *thd,
   int res;
   multi_update *result;
   TABLE_LIST *tl;
+  const bool locked= !(thd->locked_tables);
   DBUG_ENTER("mysql_multi_update");
 
-  if ((res=open_and_lock_tables(thd,table_list)))
+  for (;;)
+  {
+    table_map update_map= 0;
+    int tnr= 0;
+    
+    if ((res= open_tables(thd, table_list)))
+      DBUG_RETURN(res);
+
+    /*
+      Only need to call lock_tables if (thd->locked_tables == NULL)
+    */
+    if (locked && ((res= lock_tables(thd, table_list))))
       DBUG_RETURN(res);
 
     thd->select_limit=HA_POS_ERROR;
@@ -411,16 +423,80 @@ int mysql_multi_update(THD *thd,
     /*
       Ensure that we have update privilege for all tables and columns in the
       SET part
+      While we are here, initialize the table->map field.
     */
     for (tl= table_list ; tl ; tl=tl->next)
     {
       TABLE *table= tl->table;
       table->grant.want_privilege= (UPDATE_ACL & ~table->grant.privilege);
+      table->map= (table_map) 1 << (tnr++);
     }
 
-  if (setup_fields(thd, table_list, *fields, 1, 0, 0))
+    if (!setup_fields(thd, table_list, *fields, 1, 0, 0))
+    {
+      List_iterator_fast<Item> field_it(*fields);
+      Item_field *item;
+
+      while ((item= (Item_field *) field_it++)) 
+	update_map|= item->used_tables();
+
+      DBUG_PRINT("info",("update_map=0x%08x", update_map));
+    }
+    else
       DBUG_RETURN(-1);
 
+    /*
+      Unlock the tables in preparation for relocking
+    */
+    if (locked) 
+    {      
+      pthread_mutex_lock(&LOCK_open);
+      mysql_unlock_tables(thd, thd->lock); 
+      thd->lock= 0;
+      pthread_mutex_unlock(&LOCK_open);
+    }
+
+    /*
+      Set the table locking strategy according to the update map
+    */
+    for (tl= table_list ; tl ; tl=tl->next)
+    {
+      TABLE *table= tl->table;
+      if (update_map & table->map) 
+      {
+	DBUG_PRINT("info",("setting table `%s` for update", tl->alias));
+	tl->lock_type= thd->lex.lock_option;
+	tl->updating= 1;
+      }
+      else
+      {
+	DBUG_PRINT("info",("setting table `%s` for read-only", tl->alias));
+	tl->lock_type= TL_READ;
+	tl->updating= 0;
+      }
+      if (locked)
+	tl->table->reginfo.lock_type= tl->lock_type;
+    }
+
+    /*
+      Relock the tables
+    */
+    if (!(res=lock_tables(thd,table_list)))
+      break;
+      
+    if (!locked)
+      DBUG_RETURN(res);
+
+    List_iterator_fast<Item> field_it(*fields);
+    Item_field *item;
+
+    while ((item= (Item_field *) field_it++)) 
+    /*  item->cleanup();		XXX Use this instead in MySQL 4.1+ */
+      item->field= item->result_field= 0;
+
+    close_thread_tables(thd);
+  }
+
   /*
     Count tables and setup timestamp handling
   */

sql/sql_yacc.yy

@@ -2751,10 +2751,18 @@ update:
           lex->select->order_list.next= (byte**) &lex->select->order_list.first;
         }
         opt_low_priority opt_ignore join_table_list
-	SET update_list where_clause opt_order_clause delete_limit_clause
+	SET update_list 
 	{
+	  if (Lex->select->table_list.elements > 1)
+	  {
+	    LEX *lex=Lex;
+            lex->sql_command= SQLCOM_MULTI_UPDATE;
+	    lex->lock_option= $3;
+	  }
+	  else
 	    set_lock_for_tables($3);
 	}
+	where_clause opt_order_clause delete_limit_clause {}
 	;
 
 update_list: