Fixed on BUG#16568: Continue handler with simple CASE not working correctly

  After trying multiple inheritance (to messy and hard make it work) and
  sublassing jump_if_not (worked, but ugly), decided to on this solution
  instead:
  Inserting an abstract sp_instr_opt_meta class as parent for all instructions
  with destinations makes it possible to handle a continuation pointer for
  sp_instr_set_case_expr too.
  Note: No special test case; the fix is captured by the changed behaviour of
  bug14643_2, and bug14498_4 (formerly disabled), in sp.test.


mysql-test/r/sp.result:
  Updated results for BUG#16568 (affects results for bug14643_2 and bug14498_4)
mysql-test/t/sp.test:
  Enabled test bug14498_4 for BUG#16568.
sql/sp_head.cc:
  Changed type of some parameters and variables (sp_instr_opt_meta instead of sp_instr_jump*).
  Added consistency check of m_ip member in instructions in sp_head::show_routine_code().
  Updated print() method of, and added opt_mark() and opt_move() methods to
  sp_instr_set_case_expr, to handle the new continuation destination.
sql/sp_head.h:
  New abstract class between sp_instr and instructions with destinations, in particular
  sp_instr_set_case_expr, for continuation destination handling.
  Changed type of some parameters and variables (sp_instr_opt_meta instead of sp_instr_jump*).
  Added opt_mark(), opt_move() and set_destination() methods to
  sp_instr_set_case_expr.
sql/sql_parse.cc:
  Fixed small bug at show_routine_code() call (tested return value the wrong way).
sql/sql_yacc.yy:
  sp_instr_set_case_expr is now added to backpatch list (for the new cont. destination).

1 files changed, 64 insertions, 21 deletions

diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index ae27b910304..a62c83e0caf 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)
   {
@@ -2039,10 +2039,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;
@@ -2064,6 +2064,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)
 {
@@ -2091,6 +2095,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);
 
@@ -2515,14 +2535,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);
 }
 
@@ -3080,30 +3100,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;
+}
+
 
 /* ------------------------------------------------------------------ */