List:Commits« Previous MessageNext Message »
From:Sergey Glukhov Date:April 7 2011 12:21pm
Subject:Re: bzr commit into mysql-5.1 branch (tor.didriksen:3648) Bug#11765713
View as plain text  
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<Item>  &all_fields,COND
> *conds)
> +int opt_sum_query(THD *thd,
> +                  TABLE_LIST *tables, List<Item>  &all_fields, COND
> *conds)
>   {
>     List_iterator_fast<Item>  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<Item>  &all_fields,COND
> *conds);
> +int opt_sum_query(THD* thd,
> +                  TABLE_LIST *tables, List<Item>  &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);
>
>
>
>


Thread
bzr commit into mysql-5.1 branch (tor.didriksen:3648) Bug#11765713Tor Didriksen7 Apr
  • Re: bzr commit into mysql-5.1 branch (tor.didriksen:3648) Bug#11765713Guilhem Bichot7 Apr
    • Re: bzr commit into mysql-5.1 branch (tor.didriksen:3648) Bug#11765713Tor Didriksen8 Apr
  • Re: bzr commit into mysql-5.1 branch (tor.didriksen:3648) Bug#11765713Sergey Glukhov11 Apr
  • Re: bzr commit into mysql-5.1 branch (tor.didriksen:3648) Bug#11765713Guilhem Bichot13 Apr
    • Re: bzr commit into mysql-5.1 branch (tor.didriksen:3648) Bug#11765713Tor Didriksen14 Apr