Uninitialized Column_definition::pack_flag for ROW-type SP variables and their fields

Fixed that the Column_definition::pack_flag member corresponding to
ROW-type SP variables and their fields was not properly initialized.
This lead to sporadic test failures. Valgrind complained about jumps
depending on uninitialized value in VALGRIND builds.

This patch makes sure that sp_head::fill_spvar_definition() is always
called for ROW variables and their fields.

Additionally, fixed that a function with a scalar parameter
erroneously acceptes ROWs with one fields. Now an error is returned.

mysql-test/suite/compat/oracle/r/sp-row.result

@@ -274,8 +274,7 @@ SELECT f1(a);
 END;
 $$
 CALL p1();
-f1(a)
-NULL
+ERROR 21000: Operand should contain 1 column(s)
 DROP PROCEDURE p1;
 DROP FUNCTION f1;
 #

mysql-test/suite/compat/oracle/t/sp-row.test

@@ -351,7 +351,7 @@ BEGIN
 END;
 $$
 DELIMITER ;$$
-#--error ER_OPERAND_COLUMNS
+--error ER_OPERAND_COLUMNS
 CALL p1();
 DROP PROCEDURE p1;
 DROP FUNCTION f1;

sql/field.cc

@@ -10583,6 +10583,7 @@ Column_definition::Column_definition(THD *thd, Field *old_field,
   default_value= orig_field ? orig_field->default_value : 0;
   check_constraint= orig_field ? orig_field->check_constraint : 0;
   option_list= old_field->option_list;
+  pack_flag= 0;
 
   switch (sql_type) {
   case MYSQL_TYPE_BLOB:

sql/field.h

@@ -3844,7 +3844,7 @@ class Column_definition: public Sql_alloc
     flags(0), pack_length(0), key_length(0), unireg_check(Field::NONE),
     interval(0), charset(&my_charset_bin),
     srid(0), geom_type(Field::GEOM_GEOMETRY),
-    option_list(NULL),
+    option_list(NULL), pack_flag(0),
     vcol_info(0), default_value(0), check_constraint(0)
   {
     interval_list.empty();
@@ -3857,7 +3857,7 @@ class Column_definition: public Sql_alloc
     flags(0), pack_length(0), key_length(0), unireg_check(Field::NONE),
     interval(0), charset(&my_charset_bin),
     srid(0), geom_type(Field::GEOM_GEOMETRY),
-    option_list(NULL),
+    option_list(NULL), pack_flag(0),
     vcol_info(0), default_value(0), check_constraint(0)
   {
     interval_list.empty();

sql/sp_head.cc

@@ -398,7 +398,8 @@ sp_eval_expr(THD *thd, Item *result_item, Field *result_field,
     expr_item is now fixed, it's safe to call cmp_type()
     If result_item is NULL, then we're setting the RETURN value.
   */
-  if (!result_item && expr_item->cmp_type() == ROW_RESULT)
+  if ((!result_item || result_item->cmp_type() != ROW_RESULT) &&
+      expr_item->cmp_type() == ROW_RESULT)
   {
     my_error(ER_OPERAND_COLUMNS, MYF(0), 1);
     goto error;

sql/sp_head.h

@@ -632,6 +632,21 @@ class sp_head :private Query_arena,
     return field_def->check(thd) ||
            field_def->sp_prepare_create_field(thd, mem_root);
   }
+  bool row_fill_field_definitions(THD *thd, Row_definition_list *row)
+  {
+    /*
+      Prepare all row fields. This will (among other things)
+      - convert VARCHAR lengths from character length to octet length
+      - calculate interval lengths for SET and ENUM
+    */
+    List_iterator<Spvar_definition> it(*row);
+    for (Spvar_definition *def= it++; def; def= it++)
+    {
+      if (fill_spvar_definition(thd, def))
+        return true;
+    }
+    return false;
+  }
   /**
     Check and prepare a Column_definition for a variable or a parameter.
   */

sql/sql_lex.cc

@@ -5233,30 +5233,20 @@ bool LEX::sp_variable_declarations_finalize(THD *thd, int nvars,
       !(dflt_value_item= new (thd->mem_root) Item_null(thd)))
     return true;
 
-  if (row)
-  {
-    /*
-      Prepare all row fields. This will (among other things)
-      - convert VARCHAR lengths from character length to octet length
-      - calculate interval lengths for SET and ENUM
-      Note, we do it only one time outside of the below loop.
-      The converted list in "row" is further reused by all variable
-      declarations processed by the current call.
-      Example:
-        DECLARE
-          a, b, c ROW(x VARCHAR(10) CHARACTER SET utf8);
-        BEGIN
-          ...
-        END;
-    */
-    List_iterator<Spvar_definition> it(*row);
-    for (Spvar_definition *def= it++; def; def= it++)
-    {
-      def->pack_flag|= FIELDFLAG_MAYBE_NULL;
-      if (sphead->fill_field_definition(thd, def))
-        return true;
-    }
-  }
+  /*
+    Prepare all row fields.
+    Note, we do it only one time outside of the below loop.
+    The converted list in "row" is further reused by all variable
+    declarations processed by the current call.
+    Example:
+      DECLARE
+        a, b, c ROW(x VARCHAR(10) CHARACTER SET utf8);
+      BEGIN
+        ...
+      END;
+  */
+  if (row && sphead->row_fill_field_definitions(thd, row))
+    return true;
 
   for (uint i= num_vars - nvars ; i < num_vars ; i++)
   {
@@ -5273,9 +5263,9 @@ bool LEX::sp_variable_declarations_finalize(THD *thd, int nvars,
     {
       if (!last)
         spvar->field_def.set_column_definition(cdef);
-      if (sphead->fill_spvar_definition(thd, &spvar->field_def, spvar->name.str))
-        return true;
     }
+    if (sphead->fill_spvar_definition(thd, &spvar->field_def, spvar->name.str))
+      return true;
     spvar->field_def.set_row_field_definitions(row);
 
     /* The last instruction is responsible for freeing LEX. */
@@ -5345,7 +5335,7 @@ LEX::sp_variable_declarations_rowtype_finalize(THD *thd, int nvars,
         return true;
       spvar->field_def.set_table_rowtype_ref(table_ref);
     }
-    spvar->field_def.field_name= spvar->name.str;
+    sphead->fill_spvar_definition(thd, &spvar->field_def, spvar->name.str);
     spvar->default_value= def;
     /* The last instruction is responsible for freeing LEX. */
     sp_instr_set *is= new (this->thd->mem_root)
@@ -5442,7 +5432,7 @@ LEX::sp_add_for_loop_cursor_variable(THD *thd,
 {
   sp_variable *spvar= spcont->add_variable(thd, name);
   spcont->declare_var_boundary(1);
-  spvar->field_def.field_name= spvar->name.str;
+  sphead->fill_spvar_definition(thd, &spvar->field_def, spvar->name.str);
   spvar->default_value= new (thd->mem_root) Item_null(thd);
 
   Cursor_rowtype *ref;

sql/sql_yacc.yy

@@ -2859,6 +2859,8 @@ sp_param_name_and_type:
           {
             $$= $1;
             $$->field_def.field_name= $$->name.str;
+            Lex->sphead->fill_spvar_definition(thd, &$$->field_def);
+            Lex->sphead->row_fill_field_definitions(thd, $2);
             $$->field_def.set_row_field_definitions($2);
           }
         ;

sql/sql_yacc_ora.yy

@@ -2339,6 +2339,8 @@ sp_param_name_and_type:
           {
             $$= $1;
             $$->field_def.field_name= $$->name.str;
+            Lex->sphead->fill_spvar_definition(thd, &$$->field_def);
+            Lex->sphead->row_fill_field_definitions(thd, $2);
             $$->field_def.set_row_field_definitions($2);
           }
         ;
@@ -2369,6 +2371,8 @@ sp_pdparam:
           {
             $1->mode= $2;
             $1->field_def.field_name= $1->name.str;
+            Lex->sphead->fill_spvar_definition(thd, &$1->field_def);
+            Lex->sphead->row_fill_field_definitions(thd, $3);
             $1->field_def.set_row_field_definitions($3);
           }
         ;