From e9452db1c1b7fb534a44590312d6608640675350 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 21 Jun 2006 01:50:20 +0400 Subject: [PATCH 1/3] Fix for bug#19634 "Re-execution of multi-delete which involve trigger/stored function crashes server". Attempts to execute prepared multi-delete statement which involved trigger or stored function caused server crashes (the same happened for such statements included in stored procedures in cases when one tried to execute them more than once). The problem was caused by yet another incorrect usage of check_table_access() routine (the latter assumes that table list which it gets as argument corresponds to value LEX::query_tables_own_last). We solve this problem by juggling with LEX::query_tables_own_last value when we call check_table_access() for LEX::auxilliary_table_list (better solution is too intrusive and should be done in 5.1). mysql-test/r/sp-prelocking.result: Added test for bug#19634 "Re-execution of multi-delete which involve trigger/ stored function crashes server". mysql-test/t/sp-prelocking.test: Added test for bug#19634 "Re-execution of multi-delete which involve trigger/ stored function crashes server". sql/sql_parse.cc: To call safely check_table_access() for LEX::auxilliary_table_list we have to juggle with LEX::query_tables_own_last value. --- mysql-test/r/sp-prelocking.result | 18 ++++++++++++++ mysql-test/t/sp-prelocking.test | 31 ++++++++++++++++++++++++ sql/sql_parse.cc | 40 +++++++++++++++++++++++++++---- 3 files changed, 85 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/sp-prelocking.result b/mysql-test/r/sp-prelocking.result index 2335513b28a..7d8dd862748 100644 --- a/mysql-test/r/sp-prelocking.result +++ b/mysql-test/r/sp-prelocking.result @@ -237,3 +237,21 @@ deallocate prepare stmt; drop table t1; drop view v1, v2, v3; drop function bug15683; +drop table if exists t1, t2, t3; +drop function if exists bug19634; +create table t1 (id int, data int); +create table t2 (id int); +create table t3 (data int); +create function bug19634() returns int return (select count(*) from t3); +prepare stmt from "delete t1 from t1, t2 where t1.id = t2.id and bug19634()"; +execute stmt; +execute stmt; +deallocate prepare stmt; +create trigger t1_bi before delete on t1 for each row insert into t3 values (old.data); +prepare stmt from "delete t1 from t1, t2 where t1.id = t2.id"; +execute stmt; +execute stmt; +deallocate prepare stmt; +drop function bug19634; +drop table t1, t2, t3; +End of 5.0 tests diff --git a/mysql-test/t/sp-prelocking.test b/mysql-test/t/sp-prelocking.test index a7215462afb..b94de6236d3 100644 --- a/mysql-test/t/sp-prelocking.test +++ b/mysql-test/t/sp-prelocking.test @@ -272,3 +272,34 @@ drop table t1; drop view v1, v2, v3; drop function bug15683; + +# +# Bug#19634 "Re-execution of multi-delete which involve trigger/stored +# function crashes server" +# +--disable_warnings +drop table if exists t1, t2, t3; +drop function if exists bug19634; +--enable_warnings +create table t1 (id int, data int); +create table t2 (id int); +create table t3 (data int); +create function bug19634() returns int return (select count(*) from t3); +prepare stmt from "delete t1 from t1, t2 where t1.id = t2.id and bug19634()"; +# This should not crash server +execute stmt; +execute stmt; +deallocate prepare stmt; + +create trigger t1_bi before delete on t1 for each row insert into t3 values (old.data); +prepare stmt from "delete t1 from t1, t2 where t1.id = t2.id"; + +execute stmt; +execute stmt; +deallocate prepare stmt; + +drop function bug19634; +drop table t1, t2, t3; + + +--echo End of 5.0 tests diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index ba5c2ebf484..7ed96250240 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -5202,8 +5202,26 @@ bool check_global_access(THD *thd, ulong want_access) /* - Check the privilege for all used tables. Table privileges are cached - in the table list for GRANT checking + Check the privilege for all used tables. + + SYNOPSYS + check_table_access() + thd Thread context + want_access Privileges requested + tables List of tables to be checked + no_errors FALSE/TRUE - report/don't report error to + the client (using my_error() call). + + NOTES + Table privileges are cached in the table list for GRANT checking. + This functions assumes that table list used and + thd->lex->query_tables_own_last value correspond to each other + (the latter should be either 0 or point to next_global member + of one of elements of this table list). + + RETURN VALUE + FALSE - OK + TRUE - Access denied */ bool @@ -7068,14 +7086,28 @@ bool multi_delete_precheck(THD *thd, TABLE_LIST *tables) SELECT_LEX *select_lex= &thd->lex->select_lex; TABLE_LIST *aux_tables= (TABLE_LIST *)thd->lex->auxilliary_table_list.first; + TABLE_LIST **save_query_tables_own_last= thd->lex->query_tables_own_last; DBUG_ENTER("multi_delete_precheck"); /* sql_yacc guarantees that tables and aux_tables are not zero */ DBUG_ASSERT(aux_tables != 0); if (check_db_used(thd, tables) || check_db_used(thd,aux_tables) || - check_table_access(thd,SELECT_ACL, tables,0) || - check_table_access(thd,DELETE_ACL, aux_tables,0)) + check_table_access(thd, SELECT_ACL, tables, 0)) DBUG_RETURN(TRUE); + + /* + Since aux_tables list is not part of LEX::query_tables list we + have to juggle with LEX::query_tables_own_last value to be able + call check_table_access() safely. + */ + thd->lex->query_tables_own_last= 0; + if (check_table_access(thd, DELETE_ACL, aux_tables, 0)) + { + thd->lex->query_tables_own_last= save_query_tables_own_last; + DBUG_RETURN(TRUE); + } + thd->lex->query_tables_own_last= save_query_tables_own_last; + if ((thd->options & OPTION_SAFE_UPDATES) && !select_lex->where) { my_message(ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE, From 15ac64063197f00a7343fb99613554788cca10b0 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 22 Jun 2006 19:15:03 +0400 Subject: [PATCH 2/3] Bug#15811: extremely long time for mysql client to execute long INSERT The problem was in redundant calls to strlen() in string functions, where we may then return after checking only the small number of characters. No test case is provided since it's a performance fix. strings/ctype-mb.c: Do not use strlen() where arbitrary horizon of at least CHARSET_INFO::mbmaxlen character is sufficient. --- strings/ctype-mb.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/strings/ctype-mb.c b/strings/ctype-mb.c index a3e10ba7650..0d73c7d1e51 100644 --- a/strings/ctype-mb.c +++ b/strings/ctype-mb.c @@ -24,12 +24,12 @@ void my_caseup_str_mb(CHARSET_INFO * cs, char *str) { register uint32 l; - register char *end=str+strlen(str); /* BAR TODO: remove strlen() call */ register uchar *map=cs->to_upper; while (*str) { - if ((l=my_ismbchar(cs, str,end))) + /* Pointing after the '\0' is safe here. */ + if ((l=my_ismbchar(cs, str, str + cs->mbmaxlen))) str+=l; else { @@ -42,12 +42,12 @@ void my_caseup_str_mb(CHARSET_INFO * cs, char *str) void my_casedn_str_mb(CHARSET_INFO * cs, char *str) { register uint32 l; - register char *end=str+strlen(str); register uchar *map=cs->to_lower; while (*str) { - if ((l=my_ismbchar(cs, str,end))) + /* Pointing after the '\0' is safe here. */ + if ((l=my_ismbchar(cs, str, str + cs->mbmaxlen))) str+=l; else { @@ -101,15 +101,18 @@ uint my_casedn_mb(CHARSET_INFO * cs, char *src, uint srclen, return srclen; } +/* + my_strcasecmp_mb() returns 0 if strings are equal, non-zero otherwise. + */ int my_strcasecmp_mb(CHARSET_INFO * cs,const char *s, const char *t) { register uint32 l; - register const char *end=s+strlen(s); register uchar *map=cs->to_upper; - while (smbmaxlen))) { while (l--) if (*s++ != *t++) @@ -120,7 +123,8 @@ int my_strcasecmp_mb(CHARSET_INFO * cs,const char *s, const char *t) else if (map[(uchar) *s++] != map[(uchar) *t++]) return 1; } - return *t; + /* At least one of '*s' and '*t' is zero here. */ + return (*t != *s); } From 67fd3c4a53f585f8e33b5094822cf639a27483de Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 22 Jun 2006 19:29:48 +0400 Subject: [PATCH 3/3] A fix and a test case for Bug#15217 "Using a SP cursor on a table created with PREPARE fails with weird error". More generally, re-executing a stored procedure with a complex SP cursor query could lead to a crash. The cause of the problem was that SP cursor queries were not optimized properly at first execution: their parse tree belongs to sp_instr_cpush, not sp_instr_copen, and thus the tree was tagged "EXECUTED" when the cursor was declared, not when it was opened. This led to loss of optimization transformations performed at first execution, as sp_instr_copen saw that the query is already "EXECUTED" and therefore either not ran first-execution related blocks or wrongly rolled back the transformations caused by first-execution code. The fix is to update the state of the parsed tree only when the tree is executed, as opposed to when the instruction containing the tree is executed. Assignment if i->state is moved to reset_lex_and_exec_core. mysql-test/r/sp.result: Test results fixed (Bug#15217) mysql-test/t/sp.test: Add a test case for Bug#15217 sql/sp_head.cc: Move assignment of stmt_arena->state to reset_lex_and_exec_core --- mysql-test/r/sp.result | 21 +++++++++++++++++++++ mysql-test/t/sp.test | 27 +++++++++++++++++++++++++++ sql/sp_head.cc | 4 +++- 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index ff378f1f43b..d3874c769fa 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -4990,4 +4990,25 @@ CALL bug18037_p2()| DROP FUNCTION bug18037_f1| DROP PROCEDURE bug18037_p1| DROP PROCEDURE bug18037_p2| +drop table if exists t3| +drop procedure if exists bug15217| +create table t3 as select 1| +create procedure bug15217() +begin +declare var1 char(255); +declare cur1 cursor for select * from t3; +open cur1; +fetch cur1 into var1; +select concat('data was: /', var1, '/'); +close cur1; +end | +call bug15217()| +concat('data was: /', var1, '/') +data was: /1/ +flush tables | +call bug15217()| +concat('data was: /', var1, '/') +data was: /1/ +drop table t3| +drop procedure bug15217| drop table t1,t2; diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 1d21a5da187..66498198157 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -5888,6 +5888,33 @@ DROP FUNCTION bug18037_f1| DROP PROCEDURE bug18037_p1| DROP PROCEDURE bug18037_p2| +# +# Bug#15217 "Using a SP cursor on a table created with PREPARE fails with +# weird error". Check that the code that is supposed to work at +# the first execution of a stored procedure actually works for +# sp_instr_copen. + +--disable_warnings +drop table if exists t3| +drop procedure if exists bug15217| +--enable_warnings +create table t3 as select 1| +create procedure bug15217() +begin + declare var1 char(255); + declare cur1 cursor for select * from t3; + open cur1; + fetch cur1 into var1; + select concat('data was: /', var1, '/'); + close cur1; +end | +# Returns expected result +call bug15217()| +flush tables | +# Returns error with garbage as column name +call bug15217()| +drop table t3| +drop procedure bug15217| # # BUG#NNNN: New bug synopsis diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 3b29a841966..ef2f895c8b2 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -1075,7 +1075,6 @@ sp_head::execute(THD *thd) thd->net.no_send_error= 0; if (i->free_list) cleanup_items(i->free_list); - i->state= Query_arena::EXECUTED; /* If we've set thd->user_var_events_alloc to mem_root of this SP @@ -2210,6 +2209,9 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp, m_lex->mark_as_requiring_prelocking(NULL); } thd->rollback_item_tree_changes(); + /* Update the state of the active arena. */ + thd->stmt_arena->state= Query_arena::EXECUTED; + /* Unlike for PS we should not call Item's destructors for newly created