revision-id: fa98ae28f5e7025e0cd1ca19c71863fd083570b7 (mariadb-10.2.14-79-gfa98ae28f5e) parent(s): 46e0c8a921978e7b95adf3f17f3c85b18d9f9ef6 author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2018-06-14 17:40:10 +0200 message: Postreview fix --- sql/item_cmpfunc.cc | 3 +-- sql/item_cmpfunc.h | 2 +- sql/item_subselect.cc | 52 +++++++++++++++++++++++++++++++++------------------ sql/item_subselect.h | 14 +++++++------- sql/sql_table.cc | 11 +++++++---- 5 files changed, 50 insertions(+), 32 deletions(-) diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 46cf412e229..55840b40ed2 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -536,8 +536,7 @@ bool Item_bool_rowready_func2::fix_length_and_dec() */ if (!args[0] || !args[1]) return FALSE; - bool res= setup_args_and_comparator(current_thd, &cmp); - return res; + return setup_args_and_comparator(current_thd, &cmp); } diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 5efb98bc81d..0d3f20f1d63 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -915,7 +915,7 @@ class Item_func_strcmp :public Item_int_func { if (agg_arg_charsets_for_comparison(cmp_collation, args, 2)) return TRUE; - fix_char_length(2); // returns "1" or "0" or "-1" + fix_char_length(2); return FALSE; } Item *get_copy(THD *thd, MEM_ROOT *mem_root) diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index ca49e06a9e7..24121292869 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -304,11 +304,14 @@ bool Item_subselect::fix_fields(THD *thd_param, Item **ref) if (engine->cols() > max_columns) { my_error(ER_OPERAND_COLUMNS, MYF(0), 1); - + res= TRUE; goto end; } if (fix_length_and_dec()) + { + res= TRUE; goto end; + } } else goto end; @@ -907,7 +910,8 @@ Item::Type Item_subselect::type() const bool Item_subselect::fix_length_and_dec() { - engine->fix_length_and_dec(0); + if (engine->fix_length_and_dec(0)) + return TRUE; return FALSE; } @@ -1200,14 +1204,15 @@ bool Item_singlerow_subselect::fix_length_and_dec() { if ((max_columns= engine->cols()) == 1) { - engine->fix_length_and_dec(row= &value); + if (engine->fix_length_and_dec(row= &value)) + return TRUE; } else { if (!(row= (Item_cache**) current_thd->alloc(sizeof(Item_cache*) * - max_columns))) + max_columns)) || + engine->fix_length_and_dec(row)) return TRUE; - engine->fix_length_and_dec(row); value= *row; } unsigned_flag= value->unsigned_flag; @@ -1516,8 +1521,11 @@ bool Item_exists_subselect::fix_length_and_dec() We need only 1 row to determine existence (i.e. any EXISTS that is not an IN always requires LIMIT 1) */ + Item *item= new (thd->mem_root) Item_int(thd, (int32) 1); + if (!item) + DBUG_RETURN(TRUE); thd->change_item_tree(&unit->global_parameters()->select_limit, - new (thd->mem_root) Item_int(thd, (int32) 1)); + item); DBUG_PRINT("info", ("Set limit to 1")); DBUG_RETURN(FALSE); } @@ -3714,11 +3722,11 @@ bool subselect_single_select_engine::no_rows() } -/* - makes storage for the output values for the subquery and calcuates +/** + Makes storage for the output values for the subquery and calcuates their data and column types and their nullability. -*/ -void subselect_engine::set_row(List<Item> &item_list, Item_cache **row) +*/ +bool subselect_engine::set_row(List<Item> &item_list, Item_cache **row) { Item *sel_item; List_iterator_fast<Item> li(item_list); @@ -3734,44 +3742,51 @@ void subselect_engine::set_row(List<Item> &item_list, Item_cache **row) item->unsigned_flag= sel_item->unsigned_flag; maybe_null= sel_item->maybe_null; if (!(row[i]= Item_cache::get_cache(thd, sel_item, sel_item->cmp_type()))) - return; + return TRUE; row[i]->setup(thd, sel_item); //psergey-backport-timours: row[i]->store(sel_item); } if (item_list.elements > 1) cmp_type= res_type= ROW_RESULT; + return FALSE; } -void subselect_single_select_engine::fix_length_and_dec(Item_cache **row) +bool subselect_single_select_engine::fix_length_and_dec(Item_cache **row) { DBUG_ASSERT(row || select_lex->item_list.elements==1); - set_row(select_lex->item_list, row); + if (set_row(select_lex->item_list, row)) + return TRUE; item->collation.set(row[0]->collation); if (cols() != 1) maybe_null= 0; + return FALSE; } -void subselect_union_engine::fix_length_and_dec(Item_cache **row) +bool subselect_union_engine::fix_length_and_dec(Item_cache **row) { DBUG_ASSERT(row || unit->first_select()->item_list.elements==1); if (unit->first_select()->item_list.elements == 1) { - set_row(unit->types, row); + if (set_row(unit->types, row)) + return TRUE; item->collation.set(row[0]->collation); } else { bool maybe_null_saved= maybe_null; - set_row(unit->types, row); + if (set_row(unit->types, row)) + return TRUE; maybe_null= maybe_null_saved; } + return FALSE; } -void subselect_uniquesubquery_engine::fix_length_and_dec(Item_cache **row) +bool subselect_uniquesubquery_engine::fix_length_and_dec(Item_cache **row) { //this never should be called DBUG_ASSERT(0); + return FALSE; } int read_first_record_seq(JOIN_TAB *tab); @@ -5604,9 +5619,10 @@ void subselect_hash_sj_engine::print(String *str, enum_query_type query_type) )); } -void subselect_hash_sj_engine::fix_length_and_dec(Item_cache** row) +bool subselect_hash_sj_engine::fix_length_and_dec(Item_cache** row) { DBUG_ASSERT(FALSE); + return FALSE; } void subselect_hash_sj_engine::exclude() diff --git a/sql/item_subselect.h b/sql/item_subselect.h index f44a5c4d0ce..bd6a1bdc498 100644 --- a/sql/item_subselect.h +++ b/sql/item_subselect.h @@ -810,7 +810,7 @@ class subselect_engine: public Sql_alloc void set_thd(THD *thd_arg); THD * get_thd() { return thd ? thd : current_thd; } virtual int prepare(THD *)= 0; - virtual void fix_length_and_dec(Item_cache** row)= 0; + virtual bool fix_length_and_dec(Item_cache** row)= 0; /* Execute the engine @@ -854,7 +854,7 @@ class subselect_engine: public Sql_alloc virtual int get_identifier() { DBUG_ASSERT(0); return 0; } virtual void force_reexecution() {} protected: - void set_row(List<Item> &item_list, Item_cache **row); + bool set_row(List<Item> &item_list, Item_cache **row); }; @@ -870,7 +870,7 @@ class subselect_single_select_engine: public subselect_engine Item_subselect *item); void cleanup(); int prepare(THD *thd); - void fix_length_and_dec(Item_cache** row); + bool fix_length_and_dec(Item_cache** row); int exec(); uint cols(); uint8 uncacheable(); @@ -905,7 +905,7 @@ class subselect_union_engine: public subselect_engine Item_subselect *item); void cleanup(); int prepare(THD *); - void fix_length_and_dec(Item_cache** row); + bool fix_length_and_dec(Item_cache** row); int exec(); uint cols(); uint8 uncacheable(); @@ -963,7 +963,7 @@ class subselect_uniquesubquery_engine: public subselect_engine ~subselect_uniquesubquery_engine(); void cleanup(); int prepare(THD *); - void fix_length_and_dec(Item_cache** row); + bool fix_length_and_dec(Item_cache** row); int exec(); uint cols() { return 1; } uint8 uncacheable() { return UNCACHEABLE_DEPENDENT_INJECTED; } @@ -1112,7 +1112,7 @@ class subselect_hash_sj_engine : public subselect_engine TODO: factor out all these methods in a base subselect_index_engine class because all of them have dummy implementations and should never be called. */ - void fix_length_and_dec(Item_cache** row);//=>base class + bool fix_length_and_dec(Item_cache** row);//=>base class void exclude(); //=>base class //=>base class bool change_result(Item_subselect *si, @@ -1385,7 +1385,7 @@ class subselect_partial_match_engine : public subselect_engine uint count_columns_with_nulls_arg); int prepare(THD *thd_arg) { set_thd(thd_arg); return 0; } int exec(); - void fix_length_and_dec(Item_cache**) {} + bool fix_length_and_dec(Item_cache**) { return FALSE; } uint cols() { /* TODO: what is the correct value? */ return 1; } uint8 uncacheable() { return UNCACHEABLE_DEPENDENT; } void exclude() {} diff --git a/sql/sql_table.cc b/sql/sql_table.cc index f3cb85f01d3..e010a667590 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -4895,10 +4895,9 @@ int create_table_impl(THD *thd, alter_info, create_table_mode, key_info, key_count, frm); /* - We have to check thd->is_error() here because it can be set by - Item::val* for example, and before it will be cought accidentally by - Item_func::fix_fields() of the next call. Now we removed the check - from Item_func::fix_fields() + TODO: remove this check of thd->is_error() (now it intercept + errors in some val_*() methoids and bring some single place to + such error interception). */ if (!file || thd->is_error()) goto err; @@ -7380,6 +7379,10 @@ static bool mysql_inplace_alter_table(THD *thd, /* Replace the old .FRM with the new .FRM, but keep the old name for now. Rename to the new name (if needed) will be handled separately below. + + TODO: remove this check of thd->is_error() (now it intercept + errors in some val_*() methoids and bring some single place to + such error interception). */ if (mysql_rename_table(db_type, alter_ctx->new_db, alter_ctx->tmp_name, alter_ctx->db, alter_ctx->alias,