diff options
| -rw-r--r-- | mysql-test/r/sp.result | 7 | ||||
| -rw-r--r-- | mysql-test/t/sp.test | 5 | ||||
| -rw-r--r-- | sql/sp_head.cc | 85 | ||||
| -rw-r--r-- | sql/sp_head.h | 96 | ||||
| -rw-r--r-- | sql/sql_parse.cc | 2 | ||||
| -rw-r--r-- | sql/sql_yacc.yy | 17 |
6 files changed, 149 insertions, 63 deletions
diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index 90020573df3..bec2f049bc4 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -4089,8 +4089,6 @@ NULL 1 call bug14643_2()| Handler boo -2 -2 Handler boo drop procedure bug14643_1| @@ -4432,6 +4430,11 @@ Handler error End done +call bug14498_4()| +Handler +error +End +done call bug14498_5()| Handler error diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 32299ad7e0b..53c15ffd05b 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -5210,10 +5210,7 @@ end| call bug14498_1()| call bug14498_2()| call bug14498_3()| -# We couldn't call this before, due to a known bug (BUG#14643) -# QQ We still can't since the new set_case_expr instruction breaks -# the semantics of case; it won't crash, but will get the wrong result. -#call bug14498_4()| +call bug14498_4()| call bug14498_5()| drop procedure bug14498_1| diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 173c35328b1..27dc0103335 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -1761,7 +1761,7 @@ sp_head::fill_field_definition(THD *thd, LEX *lex, void -sp_head::new_cont_backpatch(sp_instr_jump_if_not *i) +sp_head::new_cont_backpatch(sp_instr_opt_meta *i) { m_cont_level+= 1; if (i) @@ -1773,7 +1773,7 @@ sp_head::new_cont_backpatch(sp_instr_jump_if_not *i) } void -sp_head::add_cont_backpatch(sp_instr_jump_if_not *i) +sp_head::add_cont_backpatch(sp_instr_opt_meta *i) { i->m_cont_dest= m_cont_level; (void)m_cont_backpatch.push_front(i); @@ -1784,7 +1784,7 @@ sp_head::do_cont_backpatch() { uint dest= instructions(); uint lev= m_cont_level--; - sp_instr_jump_if_not *i; + sp_instr_opt_meta *i; while ((i= m_cont_backpatch.head()) && i->m_cont_dest == lev) { @@ -2048,10 +2048,10 @@ void sp_head::optimize() set_dynamic(&m_instr, (gptr)&i, dst); while ((ibp= li++)) - { - sp_instr_jump *ji= static_cast<sp_instr_jump *>(ibp); - ji->set_destination(src, dst); - } + { + sp_instr_opt_meta *im= static_cast<sp_instr_opt_meta *>(ibp); + im->set_destination(src, dst); + } } i->opt_move(dst, &bp); src+= 1; @@ -2073,6 +2073,10 @@ sp_head::opt_mark(uint ip) #ifndef DBUG_OFF +/* + Return the routine instructions as a result set. + Returns 0 if ok, !=0 on error. +*/ int sp_head::show_routine_code(THD *thd) { @@ -2100,6 +2104,22 @@ sp_head::show_routine_code(THD *thd) for (ip= 0; (i = get_instr(ip)) ; ip++) { + /* + Consistency check. If these are different something went wrong + during optimization. + */ + if (ip != i->m_ip) + { + const char *format= "Instruction at position %u has m_ip=%u"; + char tmp[sizeof(format) + 2*SP_INSTR_UINT_MAXLEN + 1]; + + sprintf(tmp, format, ip, i->m_ip); + /* + Since this is for debugging purposes only, we don't bother to + introduce a special error code for it. + */ + push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_UNKNOWN_ERROR, tmp); + } protocol->prepare_for_resend(); protocol->store((longlong)ip); @@ -2524,14 +2544,14 @@ sp_instr_jump_if_not::exec_core(THD *thd, uint *nextp) void sp_instr_jump_if_not::print(String *str) { - /* jump_if_not dest ... */ + /* jump_if_not dest(cont) ... */ if (str->reserve(2*SP_INSTR_UINT_MAXLEN+14+32)) // Add some for the expr. too return; str->qs_append(STRING_WITH_LEN("jump_if_not ")); str->qs_append(m_dest); - str->append('('); + str->qs_append('('); str->qs_append(m_cont_dest); - str->append(") "); + str->qs_append(STRING_WITH_LEN(") ")); m_expr->print(str); } @@ -3077,30 +3097,53 @@ sp_instr_set_case_expr::exec_core(THD *thd, uint *nextp) spcont->clear_handler(); thd->spcont= spcont; } + *nextp= m_cont_dest; /* For continue handler */ } + else + *nextp= m_ip+1; - *nextp = m_ip+1; - - return res; /* no error */ + return res; } void sp_instr_set_case_expr::print(String *str) { - const char CASE_EXPR_TAG[]= "set_case_expr "; - const int CASE_EXPR_TAG_LEN= sizeof(CASE_EXPR_TAG) - 1; - const int INT_STRING_MAX_LEN= 10; - - /* We must call reserve(), because qs_append() doesn't care about memory. */ - str->reserve(CASE_EXPR_TAG_LEN + INT_STRING_MAX_LEN + 2); - - str->qs_append(CASE_EXPR_TAG, CASE_EXPR_TAG_LEN); + /* set_case_expr (cont) id ... */ + str->reserve(2*SP_INSTR_UINT_MAXLEN+18+32); // Add some extra for expr too + str->qs_append(STRING_WITH_LEN("set_case_expr (")); + str->qs_append(m_cont_dest); + str->qs_append(STRING_WITH_LEN(") ")); str->qs_append(m_case_expr_id); str->qs_append(' '); m_case_expr->print(str); } +uint +sp_instr_set_case_expr::opt_mark(sp_head *sp) +{ + sp_instr *i; + + marked= 1; + 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_set_case_expr::opt_move(uint dst, List<sp_instr> *bp) +{ + if (m_cont_dest > m_ip) + bp->push_back(this); // Forward + else if (m_cont_optdest) + m_cont_dest= m_cont_optdest->m_ip; // Backward + m_ip= dst; +} + /* ------------------------------------------------------------------ */ diff --git a/sql/sp_head.h b/sql/sp_head.h index 89e86badc09..a4dd68ee4a3 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -41,6 +41,7 @@ sp_get_flags_for_command(LEX *lex); struct sp_label; class sp_instr; +class sp_instr_opt_meta; class sp_instr_jump_if_not; struct sp_cond_type; struct sp_pvar; @@ -271,11 +272,11 @@ public: // 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); + new_cont_backpatch(sp_instr_opt_meta *i); // Add an instruction to the current level void - add_cont_backpatch(sp_instr_jump_if_not *i); + add_cont_backpatch(sp_instr_opt_meta *i); // Backpatch (and pop) the current level to the current position. void @@ -372,15 +373,15 @@ private: } bp_t; List<bp_t> m_backpatch; // Instructions needing backpatching /* - We need a special list for backpatching of conditional jump's continue + We need a special list for backpatching of instructions with a 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 + mechanism - e.g. jump_if_not instructions have two different destinations 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; + List<sp_instr_opt_meta> m_cont_backpatch; uint m_cont_level; // The current cont. backpatch level /* @@ -677,21 +678,55 @@ private: }; // class sp_instr_trigger_field : public sp_instr -class sp_instr_jump : public sp_instr +/* + An abstract class for all instructions with destinations that + needs to be updated by the optimizer. + Even if not all subclasses will use both the normal destination and + the continuation destination, we put them both here for simplicity. + */ +class sp_instr_opt_meta : public sp_instr +{ +public: + + uint m_dest; // Where we will go + uint m_cont_dest; // Where continue handlers will go + + sp_instr_opt_meta(uint ip, sp_pcontext *ctx) + : sp_instr(ip, ctx), + m_dest(0), m_cont_dest(0), m_optdest(0), m_cont_optdest(0) + {} + + sp_instr_opt_meta(uint ip, sp_pcontext *ctx, uint dest) + : sp_instr(ip, ctx), + m_dest(dest), m_cont_dest(0), m_optdest(0), m_cont_optdest(0) + {} + + virtual ~sp_instr_opt_meta() + {} + + virtual void set_destination(uint old_dest, uint new_dest) + = 0; + +protected: + + sp_instr *m_optdest; // Used during optimization + sp_instr *m_cont_optdest; // Used during optimization + +}; // class sp_instr_opt_meta : public sp_instr + +class sp_instr_jump : public sp_instr_opt_meta { sp_instr_jump(const sp_instr_jump &); /* Prevent use of these */ void operator=(sp_instr_jump &); public: - uint m_dest; // Where we will go - sp_instr_jump(uint ip, sp_pcontext *ctx) - : sp_instr(ip, ctx), m_dest(0), m_optdest(0) + : sp_instr_opt_meta(ip, ctx) {} sp_instr_jump(uint ip, sp_pcontext *ctx, uint dest) - : sp_instr(ip, ctx), m_dest(dest), m_optdest(0) + : sp_instr_opt_meta(ip, ctx, dest) {} virtual ~sp_instr_jump() @@ -722,11 +757,7 @@ public: m_dest= new_dest; } -protected: - - sp_instr *m_optdest; // Used during optimization - -}; // class sp_instr_jump : public sp_instr +}; // class sp_instr_jump : public sp_instr_opt_meta class sp_instr_jump_if_not : public sp_instr_jump @@ -736,16 +767,14 @@ class sp_instr_jump_if_not : public sp_instr_jump 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_cont_dest(0), m_expr(i), - m_lex_keeper(lex, TRUE), m_cont_optdest(0) + : sp_instr_jump(ip, ctx), m_expr(i), + m_lex_keeper(lex, TRUE) {} sp_instr_jump_if_not(uint ip, sp_pcontext *ctx, Item *i, uint dest, LEX *lex) - : sp_instr_jump(ip, ctx, dest), m_cont_dest(0), m_expr(i), - m_lex_keeper(lex, TRUE), m_cont_optdest(0) + : sp_instr_jump(ip, ctx, dest), m_expr(i), + m_lex_keeper(lex, TRUE) {} virtual ~sp_instr_jump_if_not() @@ -778,7 +807,6 @@ 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 @@ -912,7 +940,7 @@ private: uint m_frame; -}; // class sp_instr_hreturn : public sp_instr +}; // class sp_instr_hreturn : public sp_instr_jump /* This is DECLARE CURSOR */ @@ -1089,14 +1117,18 @@ private: }; // class sp_instr_error : public sp_instr -class sp_instr_set_case_expr :public sp_instr +class sp_instr_set_case_expr : public sp_instr_opt_meta { public: sp_instr_set_case_expr(uint ip, sp_pcontext *ctx, uint case_expr_id, Item *case_expr, LEX *lex) - :sp_instr(ip, ctx), m_case_expr_id(case_expr_id), m_case_expr(case_expr), - m_lex_keeper(lex, TRUE) + : sp_instr_opt_meta(ip, ctx), + m_case_expr_id(case_expr_id), m_case_expr(case_expr), + m_lex_keeper(lex, TRUE) + {} + + virtual ~sp_instr_set_case_expr() {} virtual int execute(THD *thd, uint *nextp); @@ -1105,13 +1137,23 @@ public: virtual void print(String *str); + virtual uint opt_mark(sp_head *sp); + + virtual void opt_move(uint dst, List<sp_instr> *ibp); + + virtual void set_destination(uint old_dest, uint new_dest) + { + if (m_cont_dest == old_dest) + m_cont_dest= new_dest; + } + private: uint m_case_expr_id; Item *m_case_expr; sp_lex_keeper m_lex_keeper; -}; // class sp_instr_set_case_expr : public sp_instr +}; // class sp_instr_set_case_expr : public sp_instr_opt_meta #ifndef NO_EMBEDDED_ACCESS_CHECKS diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 1cbc616f63f..a61815c8202 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -4599,7 +4599,7 @@ end_with_restore_list: else sp= sp_find_routine(thd, TYPE_ENUM_FUNCTION, lex->spname, &thd->sp_func_cache, FALSE); - if (!sp || !sp->show_routine_code(thd)) + if (!sp || sp->show_routine_code(thd)) { /* We don't distinguish between errors for now */ my_error(ER_SP_DOES_NOT_EXIST, MYF(0), diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index a1ede3c45fb..ebb97cde3e4 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -2018,17 +2018,18 @@ sp_proc_stmt: sp_head *sp= lex->sphead; sp_pcontext *parsing_ctx= lex->spcont; int case_expr_id= parsing_ctx->register_case_expr(); + sp_instr_set_case_expr *i; if (parsing_ctx->push_case_expr_id(case_expr_id)) YYABORT; - - sp->add_instr( - new sp_instr_set_case_expr(sp->instructions(), - parsing_ctx, - case_expr_id, - $3, - lex)); - + + i= new sp_instr_set_case_expr(sp->instructions(), + parsing_ctx, + case_expr_id, + $3, + lex); + sp->add_cont_backpatch(i); + sp->add_instr(i); sp->m_flags|= sp_head::IN_SIMPLE_CASE; sp->restore_lex(YYTHD); } |