Hi Monty, Please find my comments below. Anything not commented on seems to be ok to me.
commit 7d9c24f6790c66d2f457837661852bf5e870b605 Author: Michael Widenius <monty@mariadb.org> Date: Tue Nov 14 07:47:58 2017 +0200
Handle failures from malloc
Most "new" failures fixed in the following files: - sql_select.cc - item.cc - opt_subselect.cc
diff --git a/sql/item.cc b/sql/item.cc index 2285ae28041..baae2a9b6fb 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -1261,7 +1261,7 @@ Item *Item::safe_charset_converter(THD *thd, CHARSET_INFO *tocs) if (!needs_charset_converter(tocs)) return this; Item_func_conv_charset *conv= new (thd->mem_root) Item_func_conv_charset(thd, this, tocs, 1); - return conv->safe ? conv : NULL; + return conv && conv->safe ? conv : NULL; }
I am not fully convinced that returning NULL from this function is called everywhere. It is of course better to return NULL and hope for the best than to crash right away.
@@ -3296,6 +3301,8 @@ void Item_field::fix_after_pullout(st_select_lex *new_parent, Item **ref, }
Name_resolution_context *ctx= new Name_resolution_context(); + if (ctx) + return; // Fatal error set if (context->select_lex == new_parent) { /*
I assume this is a typo it should be "if (!ctx)"
diff --git a/sql/sql_select.cc b/sql/sql_select.cc index abbf2616537..7e42890143b 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -713,7 +713,7 @@ JOIN::prepare(TABLE_LIST *tables_init, union_part= unit_arg->is_unit_op();
if (select_lex->handle_derived(thd->lex, DT_PREPARE)) - DBUG_RETURN(1); + DBUG_RETURN(-1);
thd->lex->current_select->context_analysis_place= NO_MATTER; thd->lex->current_select->is_item_list_lookup= 1;
I assume this is just for the sake of consistency with other DBUG_RETURN statements in this function?
@@ -14422,7 +14446,7 @@ static COND* substitute_for_best_equal_field(THD *thd, JOIN_TAB *context_tab, This works OK with PS/SP re-execution as changes are made to the arguments of AND/OR items only */ - if (new_item != item) + if (new_item && new_item != item) li.replace(new_item); }
new_item was returned from a substitute_for_best_equal_field() call. That one doesn't ever return NULL. It will return without substitution if it encounters an error. So this change is not needed.
@@ -25044,8 +25103,11 @@ int JOIN::save_explain_data_intern(Explain_query *output,
if (message) { - explain= new (output->mem_root) Explain_select(output->mem_root, - thd->lex->analyze_stmt); + if (!(explain= new (output->mem_root) + Explain_select(output->mem_root, + thd->lex->analyze_stmt))) + DBUG_RETURN(1); + join->select_lex->set_explain_type(true);
explain->select_id= join->select_lex->select_number;
The return value of this function is not checked. I assume the above statement is there so that we dont try to continue to execute the function if we've got an OOM error? In this case, probably JOIN::save_explain_data_intern() should also have a similar check for its tab->save_explain_data(eta, used_tables, distinct_arg, first_top_tab); call? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog