Below is the list of changes that have just been committed into a local
5.0 repository of pem. When pem 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.1963 05/11/04 15:37:39 pem@stripped +5 -0
Fixed BUG#14498: Stored procedures: hang if undefined variable and exception
The problem was to continue at the right place in the code after the
test expression in a flow control statement fails with an exception
(internally, the test in sp_instr_jump_if_not), and the exception is
caught by a continue handler. Execution must then be resumed after the
the entire flow control statement (END IF, END WHILE, etc).
sql/sql_yacc.yy
1.439 05/11/04 15:37:32 pem@stripped +17 -3
Added backpatching of the continue destination for all conditional statements
(using sp_instr_jump_if_not).
sql/sp_head.h
1.74 05/11/04 15:37:32 pem@stripped +47 -41
Added a continuation destination for sp_instr_jump_if_not, for the case when
an error in the test expression causes a continue handler to catch.
This includes new members in sp_instr_jump_if_not, adjustment of the optmizer
(mark and move methods), and separate backpatching code (since we can't use
the normal one for this).
Also removed the class sp_instr_jump, since it's never used.
sql/sp_head.cc
1.194 05/11/04 15:37:32 pem@stripped +74 -66
Added a continuation destination for sp_instr_jump_if_not, for the case when
an error in the test expression causes a continue handler to catch.
This includes new members in sp_instr_jump_if_not, adjustment of the optmizer
(mark and move methods), and separate backpatching code (since we can't use
the normal one for this).
Also removed the class sp_instr_jump, since it's never used.
...and added some comments to the optimizer.
mysql-test/t/sp.test
1.162 05/11/04 15:37:31 pem@stripped +86 -0
New test case for BUG#14498.
(Note that one call is disabled at the moment. Depends on BUG#14643.)
mysql-test/r/sp.result
1.169 05/11/04 15:37:31 pem@stripped +84 -0
New test case for BUG#14498.
# 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: pem
# Host: mysql.comhem.se
# Root: /usr/home/pem/bug14498/mysql-5.0
--- 1.438/sql/sql_yacc.yy 2005-10-28 12:11:24 +02:00
+++ 1.439/sql/sql_yacc.yy 2005-11-04 15:37:32 +01:00
@@ -2009,14 +2009,21 @@
}
sp->restore_lex(YYTHD);
}
- | IF sp_if END IF {}
+ | IF
+ { Lex->sphead->new_cont_backpatch(NULL); }
+ sp_if END IF
+ { Lex->sphead->do_cont_backpatch(); }
| CASE_SYM WHEN_SYM
{
Lex->sphead->m_flags&= ~sp_head::IN_SIMPLE_CASE;
+ Lex->sphead->new_cont_backpatch(NULL);
}
- sp_case END CASE_SYM {}
+ sp_case END CASE_SYM { Lex->sphead->do_cont_backpatch(); }
| CASE_SYM
- { Lex->sphead->reset_lex(YYTHD); }
+ {
+ Lex->sphead->reset_lex(YYTHD);
+ Lex->sphead->new_cont_backpatch(NULL);
+ }
expr WHEN_SYM
{
/* We "fake" this by using an anonymous variable which we
@@ -2038,6 +2045,7 @@
sp_case END CASE_SYM
{
Lex->spcont->pop_pvar();
+ Lex->sphead->do_cont_backpatch();
}
| sp_labeled_control
{}
@@ -2306,6 +2314,7 @@
$2, lex);
sp->push_backpatch(i, ctx->push_label((char *)"", 0));
+ sp->add_cont_backpatch(i);
sp->add_instr(i);
sp->restore_lex(YYTHD);
}
@@ -2360,6 +2369,7 @@
lex->variables_used= 1;
}
sp->push_backpatch(i, ctx->push_label((char *)"", 0));
+ sp->add_cont_backpatch(i);
sp->add_instr(i);
sp->restore_lex(YYTHD);
}
@@ -2489,6 +2499,7 @@
/* Jumping forward */
sp->push_backpatch(i, lex->spcont->last_label());
+ sp->new_cont_backpatch(i);
sp->add_instr(i);
sp->restore_lex(YYTHD);
}
@@ -2500,6 +2511,7 @@
sp_instr_jump *i = new sp_instr_jump(ip, lex->spcont, lab->ip);
lex->sphead->add_instr(i);
+ lex->sphead->do_cont_backpatch();
}
| REPEAT_SYM sp_proc_stmts1 UNTIL_SYM
{ Lex->sphead->reset_lex(YYTHD); }
@@ -2513,6 +2525,8 @@
lex);
lex->sphead->add_instr(i);
lex->sphead->restore_lex(YYTHD);
+ /* We can shortcut the cont_backpatch here */
+ i->m_cont_dest= ip+1;
}
;
--- 1.168/mysql-test/r/sp.result 2005-10-27 23:24:02 +02:00
+++ 1.169/mysql-test/r/sp.result 2005-11-04 15:37:31 +01:00
@@ -3575,4 +3575,88 @@
DROP PROCEDURE IF EXISTS bug13095;
DROP VIEW IF EXISTS bug13095_v1;
DROP TABLE IF EXISTS bug13095_t1;
+drop procedure if exists bug14498_1|
+drop procedure if exists bug14498_2|
+drop procedure if exists bug14498_3|
+drop procedure if exists bug14498_4|
+drop procedure if exists bug14498_5|
+create procedure bug14498_1()
+begin
+declare continue handler for sqlexception select 'error' as 'Handler';
+if v then
+select 'yes' as 'v';
+else
+select 'no' as 'v';
+end if;
+select 'done' as 'End';
+end|
+create procedure bug14498_2()
+begin
+declare continue handler for sqlexception select 'error' as 'Handler';
+while v do
+select 'yes' as 'v';
+end while;
+select 'done' as 'End';
+end|
+create procedure bug14498_3()
+begin
+declare continue handler for sqlexception select 'error' as 'Handler';
+repeat
+select 'maybe' as 'v';
+until v end repeat;
+select 'done' as 'End';
+end|
+create procedure bug14498_4()
+begin
+declare continue handler for sqlexception select 'error' as 'Handler';
+case v
+when 1 then
+select '1' as 'v';
+when 2 then
+select '2' as 'v';
+else
+select '?' as 'v';
+end case;
+select 'done' as 'End';
+end|
+create procedure bug14498_5()
+begin
+declare continue handler for sqlexception select 'error' as 'Handler';
+case
+when v = 1 then
+select '1' as 'v';
+when v = 2 then
+select '2' as 'v';
+else
+select '?' as 'v';
+end case;
+select 'done' as 'End';
+end|
+call bug14498_1()|
+Handler
+error
+End
+done
+call bug14498_2()|
+Handler
+error
+End
+done
+call bug14498_3()|
+v
+maybe
+Handler
+error
+End
+done
+call bug14498_5()|
+Handler
+error
+End
+done
+drop procedure bug14498_1|
+drop procedure bug14498_2|
+drop procedure bug14498_3|
+drop procedure bug14498_4|
+drop procedure bug14498_5|
drop table t1,t2;
--- 1.161/mysql-test/t/sp.test 2005-10-24 22:53:55 +02:00
+++ 1.162/mysql-test/t/sp.test 2005-11-04 15:37:31 +01:00
@@ -4490,6 +4490,92 @@
#
+# BUG#14498: Stored procedures: hang if undefined variable and exception
+#
+--disable_warnings
+drop procedure if exists bug14498_1|
+drop procedure if exists bug14498_2|
+drop procedure if exists bug14498_3|
+drop procedure if exists bug14498_4|
+drop procedure if exists bug14498_5|
+--enable_warnings
+
+create procedure bug14498_1()
+begin
+ declare continue handler for sqlexception select 'error' as 'Handler';
+
+ if v then
+ select 'yes' as 'v';
+ else
+ select 'no' as 'v';
+ end if;
+ select 'done' as 'End';
+end|
+
+create procedure bug14498_2()
+begin
+ declare continue handler for sqlexception select 'error' as 'Handler';
+
+ while v do
+ select 'yes' as 'v';
+ end while;
+ select 'done' as 'End';
+end|
+
+create procedure bug14498_3()
+begin
+ declare continue handler for sqlexception select 'error' as 'Handler';
+
+ repeat
+ select 'maybe' as 'v';
+ until v end repeat;
+ select 'done' as 'End';
+end|
+
+create procedure bug14498_4()
+begin
+ declare continue handler for sqlexception select 'error' as 'Handler';
+
+ case v
+ when 1 then
+ select '1' as 'v';
+ when 2 then
+ select '2' as 'v';
+ else
+ select '?' as 'v';
+ end case;
+ select 'done' as 'End';
+end|
+
+create procedure bug14498_5()
+begin
+ declare continue handler for sqlexception select 'error' as 'Handler';
+
+ case
+ when v = 1 then
+ select '1' as 'v';
+ when v = 2 then
+ select '2' as 'v';
+ else
+ select '?' as 'v';
+ end case;
+ select 'done' as 'End';
+end|
+
+call bug14498_1()|
+call bug14498_2()|
+call bug14498_3()|
+# QQ We can't call this at the moment, due to a known bug (BUG#14643)
+#call bug14498_4()|
+call bug14498_5()|
+
+drop procedure bug14498_1|
+drop procedure bug14498_2|
+drop procedure bug14498_3|
+drop procedure bug14498_4|
+drop procedure bug14498_5|
+
+#
# BUG#NNNN: New bug synopsis
#
#--disable_warnings
--- 1.193/sql/sp_head.cc 2005-10-21 13:07:50 +02:00
+++ 1.194/sql/sp_head.cc 2005-11-04 15:37:32 +01:00
@@ -437,13 +437,14 @@
sp_head::sp_head()
:Query_arena(&main_mem_root, INITIALIZED_FOR_SP),
- m_flags(0), m_returns_cs(NULL)
+ m_flags(0), m_returns_cs(NULL), m_cont_level(0)
{
extern byte *
sp_table_key(const byte *ptr, uint *plen, my_bool first);
DBUG_ENTER("sp_head::sp_head");
m_backpatch.empty();
+ m_cont_backpatch.empty();
m_lex.empty();
hash_init(&m_sptabs, system_charset_info, 0, 0, 0, sp_table_key, 0, 0);
hash_init(&m_sroutines, system_charset_info, 0, 0, 0, sp_sroutine_key, 0, 0);
@@ -1569,6 +1570,39 @@
}
void
+sp_head::new_cont_backpatch(sp_instr_jump_if_not *i)
+{
+ m_cont_level+= 1;
+ if (i)
+ {
+ /* Use the cont. destination slot to store the level */
+ i->m_cont_dest= m_cont_level;
+ (void)m_cont_backpatch.push_front(i);
+ }
+}
+
+void
+sp_head::add_cont_backpatch(sp_instr_jump_if_not *i)
+{
+ i->m_cont_dest= m_cont_level;
+ (void)m_cont_backpatch.push_front(i);
+}
+
+void
+sp_head::do_cont_backpatch()
+{
+ uint dest= instructions();
+ uint lev= m_cont_level--;
+ sp_instr_jump_if_not *i;
+
+ while ((i= m_cont_backpatch.head()) && i->m_cont_dest == lev)
+ {
+ i->m_cont_dest= dest;
+ (void)m_cont_backpatch.pop();
+ }
+}
+
+void
sp_head::set_info(char *definer, uint definerlen,
longlong created, longlong modified,
st_sp_chistics *chistics, ulong sql_mode)
@@ -1782,7 +1816,10 @@
/*
- TODO: what does this do??
+ Do some minimal optimization of the code:
+ 1) Mark used instructions
+ 1.1) While doing this, shortcut jumps to jump instructions
+ 2) Compact the code, removing unused instructions
*/
void sp_head::optimize()
@@ -1805,7 +1842,7 @@
else
{
if (src != dst)
- {
+ { // Move the instruction and update prev. jumps
sp_instr *ibp;
List_iterator_fast<sp_instr> li(bp);
@@ -1813,8 +1850,7 @@
while ((ibp= li++))
{
sp_instr_jump *ji= static_cast<sp_instr_jump *>(ibp);
- if (ji->m_dest == src)
- ji->m_dest= dst;
+ ji->set_destination(src, dst);
}
}
i->opt_move(dst, &bp);
@@ -2145,65 +2181,6 @@
/*
- sp_instr_jump_if class functions
-*/
-
-int
-sp_instr_jump_if::execute(THD *thd, uint *nextp)
-{
- DBUG_ENTER("sp_instr_jump_if::execute");
- DBUG_PRINT("info", ("destination: %u", m_dest));
- DBUG_RETURN(m_lex_keeper.reset_lex_and_exec_core(thd, nextp, TRUE, this));
-}
-
-int
-sp_instr_jump_if::exec_core(THD *thd, uint *nextp)
-{
- Item *it;
- int res;
-
- it= sp_prepare_func_item(thd, &m_expr);
- if (!it)
- res= -1;
- else
- {
- res= 0;
- if (it->val_bool())
- *nextp = m_dest;
- else
- *nextp = m_ip+1;
- }
-
- return res;
-}
-
-void
-sp_instr_jump_if::print(String *str)
-{
- str->reserve(12);
- str->append("jump_if ");
- str->qs_append(m_dest);
- str->append(' ');
- m_expr->print(str);
-}
-
-uint
-sp_instr_jump_if::opt_mark(sp_head *sp)
-{
- sp_instr *i;
-
- marked= 1;
- if ((i= sp->get_instr(m_dest)))
- {
- m_dest= i->opt_shortcut_jump(sp, this);
- m_optdest= sp->get_instr(m_dest);
- }
- sp->opt_mark(m_dest);
- return m_ip+1;
-}
-
-
-/*
sp_instr_jump_if_not class functions
*/
@@ -2224,7 +2201,10 @@
it= sp_prepare_func_item(thd, &m_expr);
if (! it)
+ {
res= -1;
+ *nextp = m_cont_dest;
+ }
else
{
res= 0;
@@ -2241,10 +2221,12 @@
void
sp_instr_jump_if_not::print(String *str)
{
- str->reserve(16);
+ str->reserve(24);
str->append("jump_if_not ");
str->qs_append(m_dest);
- str->append(' ');
+ str->append('(');
+ str->qs_append(m_cont_dest);
+ str->append(") ");
m_expr->print(str);
}
@@ -2261,7 +2243,33 @@
m_optdest= sp->get_instr(m_dest);
}
sp->opt_mark(m_dest);
+ if ((i= sp->get_instr(m_cont_dest)))
+ {
+ m_cont_dest= i->opt_shortcut_jump(sp, this);
+ m_cont_optdest= sp->get_instr(m_cont_dest);
+ }
+ sp->opt_mark(m_cont_dest);
return m_ip+1;
+}
+
+void
+sp_instr_jump_if_not::opt_move(uint dst, List<sp_instr> *bp)
+{
+ /*
+ cont. destinations may point backwards after shortcutting jumps
+ during the mark phase. If it's still pointing forwards, only
+ push this for backpatching if sp_instr_jump::opt_move() will not
+ do it (i.e. if the m_dest points backwards).
+ */
+ if (m_cont_dest > m_ip)
+ { // Forward
+ if (m_dest < m_ip)
+ bp->push_back(this);
+ }
+ else if (m_cont_optdest)
+ m_cont_dest= m_cont_optdest->m_ip; // Backward
+ /* This will take care of m_dest and m_ip */
+ sp_instr_jump::opt_move(dst, bp);
}
--- 1.73/sql/sp_head.h 2005-09-22 22:46:49 +02:00
+++ 1.74/sql/sp_head.h 2005-11-04 15:37:32 +01:00
@@ -38,6 +38,7 @@
struct sp_label;
class sp_instr;
+class sp_instr_jump_if_not;
struct sp_cond_type;
struct sp_pvar;
@@ -240,6 +241,18 @@
int
check_backpatch(THD *thd);
+ // Start a new cont. backpatch level. If 'i' is NULL, the level is just incr.
+ void
+ new_cont_backpatch(sp_instr_jump_if_not *i);
+
+ // Add an instruction to the current level
+ void
+ add_cont_backpatch(sp_instr_jump_if_not *i);
+
+ // Backpatch (and pop) the current level to the current position.
+ void
+ do_cont_backpatch();
+
char *name(uint *lenp = 0) const
{
if (lenp)
@@ -310,6 +323,18 @@
} bp_t;
List<bp_t> m_backpatch; // Instructions needing backpatching
/*
+ We need a special list for backpatching of conditional jump's continue
+ destination (in the case of a continue handler catching an error in
+ the test), since it would otherwise interfere with the normal backpatch
+ mechanism - jump_if_not instructions have two different destination
+ which are to be patched differently.
+ Since these occur in a more restricted way (always the same "level" in
+ the code), we don't need the label.
+ */
+ List<sp_instr_jump_if_not> m_cont_backpatch;
+ uint m_cont_level; // The current cont. backpatch level
+
+ /*
Multi-set representing optimized list of tables to be locked by this
routine. Does not include tables which are used by invoked routines.
@@ -622,50 +647,17 @@
m_dest= dest;
}
-protected:
-
- sp_instr *m_optdest; // Used during optimization
-
-}; // class sp_instr_jump : public sp_instr
-
-
-class sp_instr_jump_if : public sp_instr_jump
-{
- sp_instr_jump_if(const sp_instr_jump_if &); /* Prevent use of these */
- void operator=(sp_instr_jump_if &);
-
-public:
-
- sp_instr_jump_if(uint ip, sp_pcontext *ctx, Item *i, LEX *lex)
- : sp_instr_jump(ip, ctx), m_expr(i), m_lex_keeper(lex, TRUE)
- {}
-
- sp_instr_jump_if(uint ip, sp_pcontext *ctx, Item *i, uint dest, LEX *lex)
- : sp_instr_jump(ip, ctx, dest), m_expr(i), m_lex_keeper(lex, TRUE)
- {}
-
- virtual ~sp_instr_jump_if()
- {}
-
- virtual int execute(THD *thd, uint *nextp);
-
- virtual int exec_core(THD *thd, uint *nextp);
-
- virtual void print(String *str);
-
- virtual uint opt_mark(sp_head *sp);
-
- virtual uint opt_shortcut_jump(sp_head *sp, sp_instr *start)
+ virtual void set_destination(uint old_dest, uint new_dest)
{
- return m_ip;
+ if (m_dest == old_dest)
+ m_dest= new_dest;
}
-private:
+protected:
- Item *m_expr; // The condition
- sp_lex_keeper m_lex_keeper;
+ sp_instr *m_optdest; // Used during optimization
-}; // class sp_instr_jump_if : public sp_instr_jump
+}; // class sp_instr_jump : public sp_instr
class sp_instr_jump_if_not : public sp_instr_jump
@@ -675,12 +667,16 @@
public:
+ uint m_cont_dest; // Where continue handlers will go
+
sp_instr_jump_if_not(uint ip, sp_pcontext *ctx, Item *i, LEX *lex)
- : sp_instr_jump(ip, ctx), m_expr(i), m_lex_keeper(lex, TRUE)
+ : sp_instr_jump(ip, ctx), m_cont_dest(0), m_expr(i),
+ m_lex_keeper(lex, TRUE), m_cont_optdest(0)
{}
sp_instr_jump_if_not(uint ip, sp_pcontext *ctx, Item *i, uint dest, LEX *lex)
- : sp_instr_jump(ip, ctx, dest), m_expr(i), m_lex_keeper(lex, TRUE)
+ : sp_instr_jump(ip, ctx, dest), m_cont_dest(0), m_expr(i),
+ m_lex_keeper(lex, TRUE), m_cont_optdest(0)
{}
virtual ~sp_instr_jump_if_not()
@@ -699,10 +695,20 @@
return m_ip;
}
+ virtual void opt_move(uint dst, List<sp_instr> *ibp);
+
+ virtual void set_destination(uint old_dest, uint new_dest)
+ {
+ sp_instr_jump::set_destination(old_dest, new_dest);
+ if (m_cont_dest == old_dest)
+ m_cont_dest= new_dest;
+ }
+
private:
Item *m_expr; // The condition
sp_lex_keeper m_lex_keeper;
+ sp_instr *m_cont_optdest; // Used during optimization
}; // class sp_instr_jump_if_not : public sp_instr_jump
| Thread |
|---|
| • bk commit into 5.0 tree (pem:1.1963) BUG#14498 | pem | 4 Nov |
