From: Sergey Glukhov Date: April 7 2011 12:21pm Subject: Re: bzr commit into mysql-5.1 branch (tor.didriksen:3648) Bug#11765713 List-Archive: http://lists.mysql.com/commits/135175 Message-Id: <4D9DAC33.5090107@oracle.com> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="------------000302070002000709000902" --------------000302070002000709000902 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Hi, Ok to push. PS Please fix 'Confirmed' flag in the bug report. On 04/07/2011 03:56 PM, Tor Didriksen wrote: > #At file:///export/home/didrik/repo/5.1-foo/ based on revid:georgi.kodinov@stripped > > 3648 Tor Didriksen 2011-04-07 > Bug#11765713 58705: OPTIMIZER LET ENGINE DEPEND ON UNINITIALIZED VALUES CREATED BY OPT_SUM_QU > @ mysql-test/r/subselect.result > New test case. > @ mysql-test/t/subselect.test > New test case. > @ sql/opt_sum.cc > Add thd to opt_sum_query/matching_cond/find_key_for_maxmin, > enabling them to test for errors. > Return with error code if thd->is_error() rather than continuing to read the index. > @ sql/sql_select.cc > Add thd to opt_sum_query, enabling it to test for errors. > @ sql/sql_select.h > Add thd to opt_sum_query, enabling it to test for errors. > > modified: > mysql-test/r/subselect.result > mysql-test/t/subselect.test > sql/opt_sum.cc > sql/sql_select.cc > sql/sql_select.h > === modified file 'mysql-test/r/subselect.result' > --- a/mysql-test/r/subselect.result 2010-11-08 10:55:43 +0000 > +++ b/mysql-test/r/subselect.result 2011-04-07 11:56:55 +0000 > @@ -4734,3 +4734,16 @@ SELECT * FROM t2 UNION SELECT * FROM t2 > ORDER BY (SELECT * FROM t1 WHERE MATCH(a) AGAINST ('+abc' IN BOOLEAN MODE)); > DROP TABLE t1,t2; > End of 5.1 tests > +# > +# Bug #11765713 58705: > +# OPTIMIZER LET ENGINE DEPEND ON UNINITIALIZED VALUES > +# CREATED BY OPT_SUM_QUERY > +# > +CREATE TABLE t1(a INT NOT NULL, KEY (a)); > +INSERT INTO t1 VALUES (0), (1); > +SELECT 1 FROM t1 WHERE a< SOME > +(SELECT a FROM t1 WHERE a<=> > +(SELECT a FROM t1) > +); > +ERROR 21000: Subquery returns more than 1 row > +DROP TABLE t1; > > === modified file 'mysql-test/t/subselect.test' > --- a/mysql-test/t/subselect.test 2010-11-08 10:55:43 +0000 > +++ b/mysql-test/t/subselect.test 2011-04-07 11:56:55 +0000 > @@ -3726,3 +3726,20 @@ DROP TABLE t1,t2; > --enable_result_log > > --echo End of 5.1 tests > + > +--echo # > +--echo # Bug #11765713 58705: > +--echo # OPTIMIZER LET ENGINE DEPEND ON UNINITIALIZED VALUES > +--echo # CREATED BY OPT_SUM_QUERY > +--echo # > + > +CREATE TABLE t1(a INT NOT NULL, KEY (a)); > +INSERT INTO t1 VALUES (0), (1); > + > +--error ER_SUBQUERY_NO_1_ROW > +SELECT 1 FROM t1 WHERE a< SOME > + (SELECT a FROM t1 WHERE a<=> > + (SELECT a FROM t1) > + ); > + > +DROP TABLE t1; > > === modified file 'sql/opt_sum.cc' > --- a/sql/opt_sum.cc 2010-06-11 07:38:29 +0000 > +++ b/sql/opt_sum.cc 2011-04-07 11:56:55 +0000 > @@ -50,7 +50,8 @@ > #include "mysql_priv.h" > #include "sql_select.h" > > -static bool find_key_for_maxmin(bool max_fl, TABLE_REF *ref, Field* field, > +static bool find_key_for_maxmin(THD *thd, > + bool max_fl, TABLE_REF *ref, Field* field, > COND *cond, uint *range_fl, > uint *key_prefix_length); > static int reckey_in_range(bool max_fl, TABLE_REF *ref, Field* field, > @@ -211,6 +212,7 @@ static int get_index_max_value(TABLE *ta > /** > Substitutes constants for some COUNT(), MIN() and MAX() functions. > > + @param thd thread handler > @param tables list of leaves of join table tree > @param all_fields All fields to be returned > @param conds WHERE clause > @@ -230,7 +232,8 @@ static int get_index_max_value(TABLE *ta > HA_ERR_... if a deadlock or a lock wait timeout happens, for example > */ > > -int opt_sum_query(TABLE_LIST *tables, List &all_fields,COND *conds) > +int opt_sum_query(THD *thd, > + TABLE_LIST *tables, List &all_fields, COND *conds) > { > List_iterator_fast it(all_fields); > int const_result= 1; > @@ -242,6 +245,8 @@ int opt_sum_query(TABLE_LIST *tables, Li > Item *item; > int error; > > + DBUG_ENTER("opt_sum_query"); > + > if (conds) > where_tables= conds->used_tables(); > > @@ -269,7 +274,7 @@ int opt_sum_query(TABLE_LIST *tables, Li > WHERE t2.field IS NULL; > */ > if (tl->table->map& where_tables) > - return 0; > + DBUG_RETURN(0); > } > else > used_tables|= tl->table->map; > @@ -297,7 +302,7 @@ int opt_sum_query(TABLE_LIST *tables, Li > { > tl->table->file->print_error(error, MYF(0)); > tl->table->in_use->fatal_error(); > - return error; > + DBUG_RETURN(error); > } > count*= tl->table->file->stats.records; > } > @@ -368,7 +373,7 @@ int opt_sum_query(TABLE_LIST *tables, Li > returned in range_fl. > */ > if (table->file->inited || (outer_tables& table->map) || > - !find_key_for_maxmin(is_max,&ref, item_field->field, conds, > + !find_key_for_maxmin(thd, is_max,&ref, item_field->field, conds, > &range_fl,&prefix_len)) > { > const_result= 0; > @@ -390,10 +395,10 @@ int opt_sum_query(TABLE_LIST *tables, Li > if (error) > { > if (error == HA_ERR_KEY_NOT_FOUND || error == HA_ERR_END_OF_FILE) > - return HA_ERR_KEY_NOT_FOUND; // No rows matching WHERE > + DBUG_RETURN(HA_ERR_KEY_NOT_FOUND); // No rows matching WHERE > /* HA_ERR_LOCK_DEADLOCK or some other error */ > table->file->print_error(error, MYF(0)); > - return(error); > + DBUG_RETURN(error); > } > removed_tables|= table->map; > } > @@ -437,6 +442,10 @@ int opt_sum_query(TABLE_LIST *tables, Li > const_result= 0; > } > } > + > + if (thd->is_error()) > + DBUG_RETURN(thd->main_da.sql_errno()); > + > /* > If we have a where clause, we can only ignore searching in the > tables if MIN/MAX optimisation replaced all used tables > @@ -446,7 +455,7 @@ int opt_sum_query(TABLE_LIST *tables, Li > */ > if (removed_tables&& used_tables != removed_tables) > const_result= 0; // We didn't remove all tables > - return const_result; > + DBUG_RETURN(const_result); > } > > > @@ -565,6 +574,7 @@ bool simple_pred(Item_func *func_item, I > field identifiers), which will obviously fail, leading to 5 being stored in > the Field object. > > + @param[in] thd thread handler > @param[in] max_fl Set to true if we are optimizing MAX(), > false means we are optimizing %MIN() > @param[in, out] ref Reference to the structure where the function > @@ -589,7 +599,7 @@ bool simple_pred(Item_func *func_item, I > true We can use the index to get MIN/MAX value > */ > > -static bool matching_cond(bool max_fl, TABLE_REF *ref, KEY *keyinfo, > +static bool matching_cond(THD *thd, bool max_fl, TABLE_REF *ref, KEY *keyinfo, > KEY_PART_INFO *field_part, COND *cond, > key_part_map *key_part_used, uint *range_fl, > uint *prefix_len) > @@ -613,7 +623,7 @@ static bool matching_cond(bool max_fl, T > Item *item; > while ((item= li++)) > { > - if (!matching_cond(max_fl, ref, keyinfo, field_part, item, > + if (!matching_cond(thd, max_fl, ref, keyinfo, field_part, item, > key_part_used, range_fl, prefix_len)) > DBUG_RETURN(FALSE); > } > @@ -732,6 +742,9 @@ static bool matching_cond(bool max_fl, T > > if (is_null || (is_null_safe_eq&& args[1]->is_null())) > { > + if (thd->is_error()) > + DBUG_RETURN(false); > + > part->field->set_null(); > *key_ptr= (uchar) 1; > } > @@ -794,6 +807,7 @@ static bool matching_cond(bool max_fl, T > (if we have a condition field = const, prefix_len contains the length > of the whole search key) > > + @param[in] thd thread handler > @param[in] max_fl 0 for MIN(field) / 1 for MAX(field) > @param[in,out] ref Reference to the structure we store the key value > @param[in] field Field used inside MIN() / MAX() > @@ -802,8 +816,9 @@ static bool matching_cond(bool max_fl, T > @param[out] prefix_len Length of prefix for the search range > > @note > - This function may set table->key_read to 1, which must be reset after > - index is used! (This can only happen when function returns 1) > + This function may set field->table->key_read to true, > + which must be reset after index is used! > + (This can only happen when function returns 1) > > @retval > 0 Index can not be used to optimize MIN(field)/MAX(field) > @@ -813,12 +828,14 @@ static bool matching_cond(bool max_fl, T > */ > > > -static bool find_key_for_maxmin(bool max_fl, TABLE_REF *ref, > +static bool find_key_for_maxmin(THD* thd, bool max_fl, TABLE_REF *ref, > Field* field, COND *cond, > uint *range_fl, uint *prefix_len) > { > if (!(field->flags& PART_KEY_FLAG)) > - return 0; // Not key field > + return false; // Not key field > + > + DBUG_ENTER("find_key_for_maxmin"); > > TABLE *table= field->table; > uint idx= 0; > @@ -843,7 +860,7 @@ static bool find_key_for_maxmin(bool max > part++, jdx++, key_part_to_use= (key_part_to_use<< 1) | 1) > { > if (!(table->file->index_flags(idx, jdx, 0)& HA_READ_ORDER)) > - return 0; > + DBUG_RETURN(false); > > /* Check whether the index component is partial */ > Field *part_field= table->field[part->fieldnr-1]; > @@ -858,9 +875,13 @@ static bool find_key_for_maxmin(bool max > ref->key_parts= 0; > key_part_map key_part_used= 0; > *range_fl= NO_MIN_RANGE | NO_MAX_RANGE; > - if (matching_cond(max_fl, ref, keyinfo, part, cond, > -&key_part_used, range_fl, prefix_len)&& > - !(key_part_to_use& ~key_part_used)) > + const bool cond_matches_key= > + matching_cond(thd, max_fl, ref, keyinfo, part, cond, > +&key_part_used, range_fl, prefix_len); > + if (thd->is_error()) > + DBUG_RETURN(false); > + > + if (cond_matches_key&& !(key_part_to_use& ~key_part_used)) > { > if (!max_fl&& key_part_used == key_part_to_use&& part->null_bit) > { > @@ -892,12 +913,12 @@ static bool find_key_for_maxmin(bool max > */ > if (field->part_of_key.is_set(idx)) > table->set_keyread(TRUE); > - return 1; > + DBUG_RETURN(true); > } > } > } > } > - return 0; > + DBUG_RETURN(false); > } > > > > === modified file 'sql/sql_select.cc' > --- a/sql/sql_select.cc 2011-02-22 21:03:32 +0000 > +++ b/sql/sql_select.cc 2011-04-07 11:56:55 +0000 > @@ -961,7 +961,7 @@ JOIN::optimize() > If all items were resolved by opt_sum_query, there is no need to > open any tables. > */ > - if ((res=opt_sum_query(select_lex->leaf_tables, all_fields, conds))) > + if ((res=opt_sum_query(thd, select_lex->leaf_tables, all_fields, conds))) > { > if (res == HA_ERR_KEY_NOT_FOUND) > { > > === modified file 'sql/sql_select.h' > --- a/sql/sql_select.h 2011-01-07 14:06:24 +0000 > +++ b/sql/sql_select.h 2011-04-07 11:56:55 +0000 > @@ -612,7 +612,8 @@ Field* create_tmp_field_from_field(THD * > > /* functions from opt_sum.cc */ > bool simple_pred(Item_func *func_item, Item **args, bool *inv_order); > -int opt_sum_query(TABLE_LIST *tables, List &all_fields,COND *conds); > +int opt_sum_query(THD* thd, > + TABLE_LIST *tables, List &all_fields, COND *conds); > > /* from sql_delete.cc, used by opt_range.cc */ > extern "C" int refpos_order_cmp(void* arg, const void *a,const void *b); > > > > --------------000302070002000709000902--