From 48a7ea6da3dc98f055e7aee0d05df04cfbc5855e Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Fri, 31 Mar 2017 21:20:32 +0400 Subject: [PATCH] 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. --- .../suite/compat/oracle/r/sp-row.result | 3 +- mysql-test/suite/compat/oracle/t/sp-row.test | 2 +- sql/field.cc | 1 + sql/field.h | 4 +- sql/sp_head.cc | 3 +- sql/sp_head.h | 15 ++++++ sql/sql_lex.cc | 46 ++++++++----------- sql/sql_yacc.yy | 2 + sql/sql_yacc_ora.yy | 4 ++ 9 files changed, 46 insertions(+), 34 deletions(-) diff --git a/mysql-test/suite/compat/oracle/r/sp-row.result b/mysql-test/suite/compat/oracle/r/sp-row.result index d515fc9e11a..5712bce3cee 100644 --- a/mysql-test/suite/compat/oracle/r/sp-row.result +++ b/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; # diff --git a/mysql-test/suite/compat/oracle/t/sp-row.test b/mysql-test/suite/compat/oracle/t/sp-row.test index e1b22eacd91..bf705022038 100644 --- a/mysql-test/suite/compat/oracle/t/sp-row.test +++ b/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; diff --git a/sql/field.cc b/sql/field.cc index 06cf9fbee58..812c4dbb639 100644 --- a/sql/field.cc +++ b/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: diff --git a/sql/field.h b/sql/field.h index 4f29619fa85..448eee9b04d 100644 --- a/sql/field.h +++ b/sql/field.h @@ -3844,7 +3844,7 @@ public: 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 @@ public: 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(); diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 20a0467be71..d819112dbfc 100644 --- a/sql/sp_head.cc +++ b/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; diff --git a/sql/sp_head.h b/sql/sp_head.h index 3dbed697f75..28b94732b5c 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -632,6 +632,21 @@ public: 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 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. */ diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 5f764f625f6..dca203863e2 100644 --- a/sql/sql_lex.cc +++ b/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 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; diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 352fd5c725a..c2f6157da52 100644 --- a/sql/sql_yacc.yy +++ b/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); } ; diff --git a/sql/sql_yacc_ora.yy b/sql/sql_yacc_ora.yy index 716f2e3ad0a..de0cb80cd84 100644 --- a/sql/sql_yacc_ora.yy +++ b/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); } ;