Fixed BUG#2653: Undeclared variables not detected in stored procedures.

We now get an run-time error instead of a crash (although a slightly misleading
error message, but it's an improvement).


mysql-test/r/sp-error.result:
  New test case for BUG#2653.
mysql-test/t/sp-error.test:
  New test case for BUG#2653.
sql/sp_head.cc:
  Detect failed evals (fix item really), which are due to unresolved variables/fields.
  Typically this would be a reference to an undeclared variable.
  (Also got rid of some compiler warnings.)
sql/sp_rcontext.cc:
  Detect failed evals (fix item really), which are due to unresolved variables/fields.
  Typically this would be a reference to an undeclared variable.
sql/sp_rcontext.h:
  Changed return type to int, so be able to detect failed evals (fix item).
sql/sql_class.cc:
  Changed return type to int, so be able to detect failed evals (fix item).

mysql-test/r/sp-error.result

@@ -397,4 +397,20 @@ drop procedure bug3279|
 drop table t3|
 create procedure nodb.bug3339() begin end|
 ERROR 42000: Unknown database 'nodb'
+create procedure bug2653_1(a int, out b int)
+set b = aa|
+create procedure bug2653_2(a int, out b int)
+begin
+if aa < 0 then
+set b = - a;
+else
+set b = a;
+end if;
+end|
+call bug2653_1(1, @b)|
+ERROR 42S22: Unknown column 'aa' in 'order clause'
+call bug2653_2(2, @b)|
+ERROR 42S22: Unknown column 'aa' in 'order clause'
+drop procedure bug2653_1|
+drop procedure bug2653_2|
 drop table t1|

mysql-test/t/sp-error.test

@@ -553,6 +553,28 @@ drop table t3|
 --error 1049
 create procedure nodb.bug3339() begin end|
 
+#
+# BUG#2653
+#
+create procedure bug2653_1(a int, out b int)
+  set b = aa|
+
+create procedure bug2653_2(a int, out b int)
+begin
+  if aa < 0 then
+    set b = - a;
+  else
+    set b = a;
+  end if;
+end|
+
+--error 1054
+call bug2653_1(1, @b)|
+--error 1054
+call bug2653_2(2, @b)|
+
+drop procedure bug2653_1|
+drop procedure bug2653_2|
 
 drop table t1|
 

sql/sp_head.cc

@@ -58,7 +58,7 @@ sp_eval_func_item(THD *thd, Item *it, enum enum_field_types type)
   if (!it->fixed && it->fix_fields(thd, 0, &it))
   {
     DBUG_PRINT("info", ("fix_fields() failed"));
-    DBUG_RETURN(it);		// Shouldn't happen?
+    DBUG_RETURN(NULL);
   }
 
   /* QQ How do we do this? Is there some better way? */
@@ -482,8 +482,14 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   for (i= 0 ; i < params && i < argcount ; i++)
   {
     sp_pvar_t *pvar = m_pcont->find_pvar(i);
+    Item *it= sp_eval_func_item(thd, *argp++, pvar->type);
 
-    nctx->push_item(sp_eval_func_item(thd, *argp++, pvar->type));
+    if (it)
+      nctx->push_item(it);
+    else
+    {
+      DBUG_RETURN(-1);
+    }
   }
 #ifdef NOT_WORKING
   /*
@@ -532,7 +538,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 {
   DBUG_ENTER("sp_head::execute_procedure");
   DBUG_PRINT("info", ("procedure %s", m_name.str));
-  int ret;
+  int ret= 0;
   uint csize = m_pcont->max_framesize();
   uint params = m_pcont->params();
   uint hmax = m_pcont->handlers();
@@ -577,7 +583,17 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 	  nctx->push_item(nit); // OUT
 	}
 	else
-	  nctx->push_item(sp_eval_func_item(thd, it,pvar->type)); // IN or INOUT
+	{
+	  Item *it2= sp_eval_func_item(thd, it,pvar->type);
+
+	  if (it2)
+	    nctx->push_item(it2); // IN or INOUT
+	  else
+	  {
+	    ret= -1;		// Eval failed
+	    break;
+	  }
+	}
 	// Note: If it's OUT or INOUT, it must be a variable.
 	// QQ: We can check for global variables here, or should we do it
 	//     while parsing?
@@ -602,7 +618,8 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
     thd->spcont= nctx;
   }
 
-  ret= execute(thd);
+  if (! ret)
+    ret= execute(thd);
 
   // Don't copy back OUT values if we got an error
   if (ret)
@@ -858,7 +875,9 @@ sp_head::show_create_procedure(THD *thd)
 
   DBUG_ENTER("sp_head::show_create_procedure");
   DBUG_PRINT("info", ("procedure %s", m_name.str));
-
+  LINT_INIT(sql_mode_str);
+  LINT_INIT(sql_mode_len);
+  
   old_sql_mode= thd->variables.sql_mode;
   thd->variables.sql_mode= m_sql_mode;
   sql_mode_var= find_sys_var("SQL_MODE", 8);
@@ -923,6 +942,8 @@ sp_head::show_create_function(THD *thd)
   ulong sql_mode_len;
   DBUG_ENTER("sp_head::show_create_function");
   DBUG_PRINT("info", ("procedure %s", m_name.str));
+  LINT_INIT(sql_mode_str);
+  LINT_INIT(sql_mode_len);
 
   old_sql_mode= thd->variables.sql_mode;
   thd->variables.sql_mode= m_sql_mode;
@@ -1025,7 +1046,11 @@ sp_instr_set::execute(THD *thd, uint *nextp)
 {
   DBUG_ENTER("sp_instr_set::execute");
   DBUG_PRINT("info", ("offset: %u", m_offset));
-  thd->spcont->set_item(m_offset, sp_eval_func_item(thd, m_value, m_type));
+  Item *it= sp_eval_func_item(thd, m_value, m_type);
+
+  if (! it)
+    DBUG_RETURN(-1);
+  thd->spcont->set_item(m_offset, it);
   *nextp = m_ip+1;
   DBUG_RETURN(0);
 }
@@ -1071,6 +1096,8 @@ sp_instr_jump_if::execute(THD *thd, uint *nextp)
   DBUG_PRINT("info", ("destination: %u", m_dest));
   Item *it= sp_eval_func_item(thd, m_expr, MYSQL_TYPE_TINY);
 
+  if (!it)
+    DBUG_RETURN(-1);
   if (it->val_int())
     *nextp = m_dest;
   else
@@ -1098,6 +1125,8 @@ sp_instr_jump_if_not::execute(THD *thd, uint *nextp)
   DBUG_PRINT("info", ("destination: %u", m_dest));
   Item *it= sp_eval_func_item(thd, m_expr, MYSQL_TYPE_TINY);
 
+  if (! it)
+    DBUG_RETURN(-1);
   if (! it->val_int())
     *nextp = m_dest;
   else
@@ -1122,7 +1151,11 @@ int
 sp_instr_freturn::execute(THD *thd, uint *nextp)
 {
   DBUG_ENTER("sp_instr_freturn::execute");
-  thd->spcont->set_result(sp_eval_func_item(thd, m_value, m_type));
+  Item *it= sp_eval_func_item(thd, m_value, m_type);
+
+  if (! it)
+    DBUG_RETURN(-1);
+  thd->spcont->set_result(it);
   *nextp= UINT_MAX;
   DBUG_RETURN(0);
 }

sql/sp_rcontext.cc

@@ -40,12 +40,19 @@ sp_rcontext::sp_rcontext(uint fsize, uint hmax, uint cmax)
   m_saved.empty();
 }
 
-void
+int
 sp_rcontext::set_item_eval(uint idx, Item *i, enum_field_types type)
 {
   extern Item *sp_eval_func_item(THD *thd, Item *it, enum_field_types type);
+  Item *it= sp_eval_func_item(current_thd, i, type);
 
-  set_item(idx, sp_eval_func_item(current_thd, i, type));
+  if (! it)
+    return -1;
+  else
+  {
+    set_item(idx, it);
+    return 0;
+  }
 }
 
 int

sql/sp_rcontext.h

@@ -69,7 +69,8 @@ class sp_rcontext : public Sql_alloc
       m_frame[idx] = i;
   }
 
-  void
+  /* Returns 0 on success, -1 on (eval) failure */
+  int
   set_item_eval(uint idx, Item *i, enum_field_types type);
 
   inline Item *

sql/sql_class.cc

@@ -1515,7 +1515,8 @@ bool select_dumpvar::send_data(List<Item> &items)
     {
       if ((yy=var_li++)) 
       {
-	thd->spcont->set_item_eval(yy->get_offset(), item, zz->type);
+	if (thd->spcont->set_item_eval(yy->get_offset(), item, zz->type))
+	  DBUG_RETURN(1);
       }
     }
     else