Commit c69e2fc7 authored by unknown's avatar unknown

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


mysql-test/r/sp-threads.result:
  Added test for bug #9486 "Can't perform multi-update in stored procedure".
mysql-test/t/sp-threads.test:
  Added test for bug #9486 "Can't perform multi-update in stored procedure".
sql/sp_head.cc:
  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.
sql/sql_lex.h:
  Removed no longer used LEX::multi_lock_option member.
sql/sql_prepare.cc:
  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_update.cc:
  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_yacc.yy:
  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).
parent 5630f073
...@@ -23,3 +23,20 @@ select * from t1; ...@@ -23,3 +23,20 @@ select * from t1;
s1 s2 s3 s1 s2 s3
drop table t1; drop table t1;
drop procedure bug4934; 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;
...@@ -54,6 +54,37 @@ drop table t1; ...@@ -54,6 +54,37 @@ drop table t1;
drop procedure bug4934; drop procedure bug4934;
#
# 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 # BUG#NNNN: New bug synopsis
# #
......
...@@ -2026,6 +2026,12 @@ typedef struct st_sp_table ...@@ -2026,6 +2026,12 @@ typedef struct st_sp_table
LEX_STRING qname; LEX_STRING qname;
bool temp; bool temp;
TABLE_LIST *table; 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 lock_count;
uint query_lock_count; uint query_lock_count;
} SP_TABLE; } SP_TABLE;
...@@ -2097,8 +2103,8 @@ sp_head::merge_table_list(THD *thd, TABLE_LIST *table, LEX *lex_for_tmp_check) ...@@ -2097,8 +2103,8 @@ sp_head::merge_table_list(THD *thd, TABLE_LIST *table, LEX *lex_for_tmp_check)
*/ */
if ((tab= (SP_TABLE *)hash_search(&m_sptabs, (byte *)tname, tlen))) if ((tab= (SP_TABLE *)hash_search(&m_sptabs, (byte *)tname, tlen)))
{ {
if (tab->table->lock_type < table->lock_type) if (tab->lock_type < table->lock_type)
tab->table= table; // Use the table with the highest lock type tab->lock_type= table->lock_type; // Use the table with the highest lock type
tab->query_lock_count++; tab->query_lock_count++;
if (tab->query_lock_count > tab->lock_count) if (tab->query_lock_count > tab->lock_count)
tab->lock_count++; tab->lock_count++;
...@@ -2116,6 +2122,7 @@ sp_head::merge_table_list(THD *thd, TABLE_LIST *table, LEX *lex_for_tmp_check) ...@@ -2116,6 +2122,7 @@ sp_head::merge_table_list(THD *thd, TABLE_LIST *table, LEX *lex_for_tmp_check)
lex_for_tmp_check->create_info.options & HA_LEX_CREATE_TMP_TABLE) lex_for_tmp_check->create_info.options & HA_LEX_CREATE_TMP_TABLE)
tab->temp= TRUE; tab->temp= TRUE;
tab->table= table; tab->table= table;
tab->lock_type= table->lock_type;
tab->lock_count= tab->query_lock_count= 1; tab->lock_count= tab->query_lock_count= 1;
my_hash_insert(&m_sptabs, (byte *)tab); my_hash_insert(&m_sptabs, (byte *)tab);
} }
...@@ -2188,7 +2195,7 @@ sp_head::add_used_tables_to_table_list(THD *thd, ...@@ -2188,7 +2195,7 @@ sp_head::add_used_tables_to_table_list(THD *thd,
table->alias= otable->alias; table->alias= otable->alias;
table->table_name= otable->table_name; table->table_name= otable->table_name;
table->table_name_length= otable->table_name_length; 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->cacheable_table= 1;
table->prelocking_placeholder= 1; table->prelocking_placeholder= 1;
......
...@@ -732,7 +732,7 @@ typedef struct st_lex ...@@ -732,7 +732,7 @@ typedef struct st_lex
USER_RESOURCES mqh; USER_RESOURCES mqh;
ulong type; ulong type;
enum_sql_command sql_command, orig_sql_command; 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 SSL_type ssl_type; /* defined in violite.h */
enum my_lex_states next_state; enum my_lex_states next_state;
enum enum_duplicates duplicates; enum enum_duplicates duplicates;
......
...@@ -1012,11 +1012,6 @@ static int mysql_test_update(Prepared_statement *stmt, ...@@ -1012,11 +1012,6 @@ static int mysql_test_update(Prepared_statement *stmt,
DBUG_PRINT("info", ("Switch to multi-update")); DBUG_PRINT("info", ("Switch to multi-update"));
/* pass counter value */ /* pass counter value */
thd->lex->table_count= table_count; 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 */ /* convert to multiupdate */
return 2; return 2;
} }
......
...@@ -145,11 +145,6 @@ int mysql_update(THD *thd, ...@@ -145,11 +145,6 @@ int mysql_update(THD *thd,
DBUG_PRINT("info", ("Switch to multi-update")); DBUG_PRINT("info", ("Switch to multi-update"));
/* pass counter value */ /* pass counter value */
thd->lex->table_count= table_count; 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 */ /* convert to multiupdate */
return 2; return 2;
} }
...@@ -692,8 +687,10 @@ bool mysql_multi_update_prepare(THD *thd) ...@@ -692,8 +687,10 @@ bool mysql_multi_update_prepare(THD *thd)
} }
DBUG_PRINT("info",("setting table `%s` for update", tl->alias)); 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 else
{ {
...@@ -705,15 +702,15 @@ bool mysql_multi_update_prepare(THD *thd) ...@@ -705,15 +702,15 @@ bool mysql_multi_update_prepare(THD *thd)
*/ */
tl->lock_type= using_update_log ? TL_READ_NO_INSERT : TL_READ; tl->lock_type= using_update_log ? TL_READ_NO_INSERT : TL_READ;
tl->updating= 0; 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 */ /* Check access privileges for table */
if (!tl->derived && !tl->belong_to_view) if (!tl->derived && !tl->belong_to_view)
{ {
uint want_privilege= tl->updating ? UPDATE_ACL : SELECT_ACL; 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, if (check_access(thd, want_privilege,
tl->db, &tl->grant.privilege, 0, 0) || tl->db, &tl->grant.privilege, 0, 0) ||
(grant_option && check_grant(thd, want_privilege, tl, 0, 1, 0))) (grant_option && check_grant(thd, want_privilege, tl, 0, 1, 0)))
...@@ -847,7 +844,7 @@ bool mysql_multi_update(THD *thd, ...@@ -847,7 +844,7 @@ bool mysql_multi_update(THD *thd,
result, unit, select_lex); result, unit, select_lex);
delete result; delete result;
thd->abort_on_warning= 0; thd->abort_on_warning= 0;
DBUG_RETURN(TRUE); DBUG_RETURN(FALSE);
} }
......
...@@ -5969,10 +5969,7 @@ update: ...@@ -5969,10 +5969,7 @@ update:
{ {
LEX *lex= Lex; LEX *lex= Lex;
if (lex->select_lex.table_list.elements > 1) if (lex->select_lex.table_list.elements > 1)
{
lex->sql_command= SQLCOM_UPDATE_MULTI; lex->sql_command= SQLCOM_UPDATE_MULTI;
lex->multi_lock_option= $3;
}
else if (lex->select_lex.get_table_list()->derived) else if (lex->select_lex.get_table_list()->derived)
{ {
/* it is single table update and it is update of derived table */ /* it is single table update and it is update of derived table */
...@@ -5980,8 +5977,12 @@ update: ...@@ -5980,8 +5977,12 @@ update:
lex->select_lex.get_table_list()->alias, "UPDATE"); lex->select_lex.get_table_list()->alias, "UPDATE");
YYABORT; 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 {} where_clause opt_order_clause delete_limit_clause {}
; ;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment