List:Internals« Previous MessageNext Message »
From:dlenev Date:April 15 2005 6:31pm
Subject:bk commit into 5.0 tree (dlenev:1.1820) BUG#9486
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of dlenev. When dlenev does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.1820 05/04/15 20:31:47 dlenev@stripped +7 -0
  Fix for bug #9486 "Can't perform multi-update in stored procedure".
  
  New more SP-locking friendly approach to handling locks in multi-update.
  Now we mark all tables of multi-update as needing write lock at parsing
  stage and if possible downgrade lock at execution stage (For its work
  SP-locking mechanism needs to know all lock types right after parsing
  stage).

  sql/sql_yacc.yy
    1.362 05/04/15 20:31:42 dlenev@stripped +6 -5
    update:
     Now we mark all tables of multi-update as needing write lock at parsing
     stage and if possible downgrade lock at execution stage. Old approach
     (don't set lock type until execution stage) was not working well with
     SP-locking (For its work SP-locking mechanism needs to know all lock 
     types right after parsing stage).

  sql/sql_update.cc
    1.154 05/04/15 20:31:42 dlenev@stripped +8 -11
    mysql_update()/mysql_multi_update_prepare():
     Now we mark all tables of multi-update as needing write lock at parsing
     stage and if possible downgrade lock at execution stage. Old approach
     (don't set lock type until execution stage) was not working well with
     SP-locking (For its work SP-locking mechanism needs to know all lock 
     types right after parsing stage).
    
    mysql_multi_update():
      We should return FALSE if no error occurs.

  sql/sql_prepare.cc
    1.111 05/04/15 20:31:42 dlenev@stripped +0 -5
    mysql_test_update():
      We don't need to bother about LEX::multi_lock_option if we convert
      multi-update to update anymore. Since nowdays multi-update uses 
      TABLE_LIST::lock_type for specifying lock level of updated tables
      instead of LEX::multi_lock_option.

  sql/sql_lex.h
    1.175 05/04/15 20:31:42 dlenev@stripped +1 -1
    Removed no longer used LEX::multi_lock_option member.

  sql/sp_head.cc
    1.126 05/04/15 20:31:41 dlenev@stripped +10 -3
    SP_TABLE, sp_head::merge_table_list()/add_used_tables_to_table_list():
      Since some queries during their execution (e.g. multi-update)
      may change type of lock for some of their tables and thus change
      lock_type member for some of elements of table list, we should
      store type of lock in SP_TABLE struct explicitly instead of using
      lock_type member of TABLE_LIST object pointed by SP_TABLE::table.  

  mysql-test/t/sp-threads.test
    1.3 05/04/15 20:31:41 dlenev@stripped +31 -0
    Added test for bug #9486 "Can't perform multi-update in stored procedure".

  mysql-test/r/sp-threads.result
    1.2 05/04/15 20:31:41 dlenev@stripped +17 -0
    Added test for bug #9486 "Can't perform multi-update in stored procedure".

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	dlenev
# Host:	brandersnatch.localdomain
# Root:	/home/dlenev/src/mysql-5.0-bg9486

--- 1.174/sql/sql_lex.h	Tue Apr  5 15:22:05 2005
+++ 1.175/sql/sql_lex.h	Fri Apr 15 20:31:42 2005
@@ -732,7 +732,7 @@
   USER_RESOURCES mqh;
   ulong type;
   enum_sql_command sql_command, orig_sql_command;
-  thr_lock_type lock_option, multi_lock_option;
+  thr_lock_type lock_option;
   enum SSL_type ssl_type;			/* defined in violite.h */
   enum my_lex_states next_state;
   enum enum_duplicates duplicates;

--- 1.153/sql/sql_update.cc	Mon Mar 28 16:12:29 2005
+++ 1.154/sql/sql_update.cc	Fri Apr 15 20:31:42 2005
@@ -145,11 +145,6 @@
     DBUG_PRINT("info", ("Switch to multi-update"));
     /* pass counter value */
     thd->lex->table_count= table_count;
-    /*
-      give correct value to multi_lock_option, because it will be used
-      in multiupdate
-    */
-    thd->lex->multi_lock_option= table_list->lock_type;
     /* convert to multiupdate */
     return 2;
   }
@@ -692,8 +687,10 @@
       }
 
       DBUG_PRINT("info",("setting table `%s` for update", tl->alias));
-      tl->lock_type= lex->multi_lock_option;
-      tl->updating= 1;
+      /*
+        If table will be updated we should not downgrade lock for it and
+        leave it as is.
+      */
     }
     else
     {
@@ -705,15 +702,15 @@
       */
       tl->lock_type= using_update_log ? TL_READ_NO_INSERT : TL_READ;
       tl->updating= 0;
+      /* Update TABLE::lock_type accordingly. */
+      if (!tl->placeholder() && !tl->schema_table &&
!using_lock_tables)
+        tl->table->reginfo.lock_type= tl->lock_type;
     }
 
     /* Check access privileges for table */
     if (!tl->derived && !tl->belong_to_view)
     {
       uint want_privilege= tl->updating ? UPDATE_ACL : SELECT_ACL;
-      if (!using_lock_tables)
-        tl->table->reginfo.lock_type= tl->lock_type;
-
       if (check_access(thd, want_privilege,
                         tl->db, &tl->grant.privilege, 0, 0) ||
           (grant_option && check_grant(thd, want_privilege, tl, 0, 1, 0)))
@@ -847,7 +844,7 @@
                       result, unit, select_lex);
   delete result;
   thd->abort_on_warning= 0;
-  DBUG_RETURN(TRUE);
+  DBUG_RETURN(FALSE);
 }
 
 

--- 1.361/sql/sql_yacc.yy	Wed Apr 13 01:12:14 2005
+++ 1.362/sql/sql_yacc.yy	Fri Apr 15 20:31:42 2005
@@ -5969,10 +5969,7 @@
 	{
 	  LEX *lex= Lex;
           if (lex->select_lex.table_list.elements > 1)
-	  {
             lex->sql_command= SQLCOM_UPDATE_MULTI;
-	    lex->multi_lock_option= $3;
-	  }
 	  else if (lex->select_lex.get_table_list()->derived)
 	  {
 	    /* it is single table update and it is update of derived table */
@@ -5980,8 +5977,12 @@
                      lex->select_lex.get_table_list()->alias, "UPDATE");
 	    YYABORT;
 	  }
-	  else
-	    Select->set_lock_for_tables($3);
+          /*
+            In case of multi-update setting write lock for all tables may
+            be too pessimistic. We will decrease lock level if possible in
+            mysql_multi_update().
+          */
+          Select->set_lock_for_tables($3);
 	}
 	where_clause opt_order_clause delete_limit_clause {}
 	;

--- 1.1/mysql-test/r/sp-threads.result	Fri Aug  6 20:11:10 2004
+++ 1.2/mysql-test/r/sp-threads.result	Fri Apr 15 20:31:41 2005
@@ -23,3 +23,20 @@
 s1	s2	s3
 drop table t1;
 drop procedure bug4934;
+drop procedure if exists bug9486;
+drop table if exists t1, t2;
+create table t1 (id1 int, val int);
+create table t2 (id2 int);
+create procedure bug9486()
+update t1, t2 set val= 1 where id1=id2;
+call bug9486();
+lock tables t2 write;
+ call bug9486();
+show processlist;
+Id	User	Host	db	Command	Time	State	Info
+#	root	localhost	test	Sleep	#		NULL
+#	root	localhost	test	Query	#	Locked	call bug9486()
+#	root	localhost	test	Query	#	NULL	show processlist
+unlock tables;
+drop procedure bug9486;
+drop table t1, t2;

--- 1.2/mysql-test/t/sp-threads.test	Fri Mar 18 16:33:42 2005
+++ 1.3/mysql-test/t/sp-threads.test	Fri Apr 15 20:31:41 2005
@@ -55,6 +55,37 @@
 
 
 #
+# BUG #9486 "Can't perform multi-update in stored procedure"
+#
+--disable_warnings
+drop procedure if exists bug9486;
+drop table if exists t1, t2;
+--enable_warnings
+create table t1 (id1 int, val int);
+create table t2 (id2 int);
+
+create procedure bug9486()
+  update t1, t2 set val= 1 where id1=id2;
+call bug9486();
+# Let us check that SP invocation requires write lock for t2.
+connection con2root;
+lock tables t2 write;
+connection con1root;
+send call bug9486();
+connection con2root;
+--sleep 2
+# There should be call statement in locked state.
+--replace_column 1 # 6 #
+show processlist;
+unlock tables;
+connection con1root;
+reap;
+
+drop procedure bug9486;
+drop table t1, t2;
+
+
+#
 # BUG#NNNN: New bug synopsis
 #
 #--disable_warnings

--- 1.125/sql/sp_head.cc	Tue Apr  5 17:49:05 2005
+++ 1.126/sql/sp_head.cc	Fri Apr 15 20:31:41 2005
@@ -2026,6 +2026,12 @@
   LEX_STRING qname;
   bool temp;
   TABLE_LIST *table;
+  /*
+    We can't use table->lock_type as lock type for table
+    in multi-set since it can be changed by statement during
+    its execution (e.g. as this happens for multi-update).
+  */
+  thr_lock_type lock_type;
   uint lock_count;
   uint query_lock_count;
 } SP_TABLE;
@@ -2097,8 +2103,8 @@
       */
       if ((tab= (SP_TABLE *)hash_search(&m_sptabs, (byte *)tname, tlen)))
       {
-	if (tab->table->lock_type < table->lock_type)
-	  tab->table= table;	// Use the table with the highest lock type
+        if (tab->lock_type < table->lock_type)
+          tab->lock_type= table->lock_type; // Use the table with the highest lock
type
         tab->query_lock_count++;
         if (tab->query_lock_count > tab->lock_count)
           tab->lock_count++;
@@ -2116,6 +2122,7 @@
 	    lex_for_tmp_check->create_info.options & HA_LEX_CREATE_TMP_TABLE)
 	  tab->temp= TRUE;
 	tab->table= table;
+        tab->lock_type= table->lock_type;
         tab->lock_count= tab->query_lock_count= 1;
 	my_hash_insert(&m_sptabs, (byte *)tab);
       }
@@ -2188,7 +2195,7 @@
       table->alias= otable->alias;
       table->table_name= otable->table_name;
       table->table_name_length= otable->table_name_length;
-      table->lock_type= otable->lock_type;
+      table->lock_type= stab->lock_type;
       table->cacheable_table= 1;
       table->prelocking_placeholder= 1;
 

--- 1.110/sql/sql_prepare.cc	Fri Apr  1 14:02:22 2005
+++ 1.111/sql/sql_prepare.cc	Fri Apr 15 20:31:42 2005
@@ -1012,11 +1012,6 @@
       DBUG_PRINT("info", ("Switch to multi-update"));
       /* pass counter value */
       thd->lex->table_count= table_count;
-      /*
-	give correct value to multi_lock_option, because it will be used
-	in multiupdate
-      */
-      thd->lex->multi_lock_option= table_list->lock_type;
       /* convert to multiupdate */
       return 2;
     }
Thread
bk commit into 5.0 tree (dlenev:1.1820) BUG#9486dlenev15 Apr