List:Commits« Previous MessageNext Message »
From:Roy Lyseng Date:July 5 2011 7:38am
Subject:Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594
View as plain text  
Hi Gluh,

The patch in itself makes sense and is hereby approved.

However, I think that you should file a new bug for longer-term cleanup of this 
feature. As far as I can see, the information requested is still not aggregated 
in the most appropriate place.

The basic problem here is that used table information from different select_lex 
objects are not compatible and should not be aggregated into the same field.

I think that a correct fix would look something like this:

  - Aggregate used tables information for selected expressions into the 
select_lex object where the
   tables are referenced in the from clause.

  - If there is no such select_lex object (for single-table insert, update and 
delete), aggregate the
   information on the LEX object instead.

  - This information should be sufficient for proper optimization of a UNION 
query, such as:
     SELECT t11.a, t11.b FROM t11 JOIN t12 USING(c1)
     UNION
     SELECT t21.a, t22.b FROM t21 JOIN t22 USING(c2);
   In the current code, the information aggregated into lex->used_tables is [0,1].
   This is correct for the second query of the UNION, but incorrect for the 
first query.
   Hence, the desired optimization is not carried out for the first query.

  - The proposed change also means that used tables information is propagated 
correctly also for
   subqueries, but notice that the general subquery optimizations are more 
useful than this optimization
   in this case.

  - For semijoin transformation, it is not necessary to propagate used tables 
information
   from subqueries into the parent level.

Thanks,
Roy

On 21.06.11 12.03, Sergey Glukhov wrote:
> #At file:///home/gluh/MySQL/mysql-5.1/ based on
> revid:sergey.glukhov@stripped
>
>   3649 Sergey Glukhov	2011-06-21
>        Bug#11766594 59736: SELECT DISTINCT.. INCORRECT RESULT WITH DETERMINISTIC
> FUNCTION IN WHERE C
>        There is an optimization of DISTINCT in JOIN::optimize()
>        which depends on THD::used_tables value. Each SELECT statement
>        inside SP resets used_tables value(see mysql_select()) and it
>        leads to wrong result. The fix is to replace THD::used_tables
>        with LEX::used_tables.
>       @ mysql-test/r/sp.result
>          test case
>       @ mysql-test/t/sp.test
>          test case
>       @ sql/sql_base.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_class.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_class.h
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_insert.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_lex.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_lex.h
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_prepare.cc
>          THD::used_tables is replaced with LEX::used_tables
>       @ sql/sql_select.cc
>          THD::used_tables is replaced with LEX::used_tables
>
>      modified:
>        mysql-test/r/sp.result
>        mysql-test/t/sp.test
>        sql/sql_base.cc
>        sql/sql_class.cc
>        sql/sql_class.h
>        sql/sql_insert.cc
>        sql/sql_lex.cc
>        sql/sql_lex.h
>        sql/sql_prepare.cc
>        sql/sql_select.cc
> === modified file 'mysql-test/r/sp.result'
> --- a/mysql-test/r/sp.result	2010-02-09 10:30:50 +0000
> +++ b/mysql-test/r/sp.result	2011-06-21 10:03:04 +0000
> @@ -7053,6 +7053,25 @@ init_connect
>   SET @@GLOBAL.init_connect= @old_init_connect;
>   DROP PROCEDURE p2;
>   DROP PROCEDURE p5;
> +#
> +# Bug#11766594  59736: SELECT DISTINCT.. INCORRECT RESULT WITH DETERMINISTIC
> FUNCTION IN WHERE C
> +#
> +CREATE TABLE t1 (a INT, b INT, KEY(b));
> +CREATE TABLE t2 (c INT, d INT, KEY(c));
> +INSERT INTO t1 VALUES (1,1),(1,1),(1,2);
> +INSERT INTO t2 VALUES (1,1),(1,2);
> +CREATE FUNCTION f1() RETURNS INT DETERMINISTIC
> +BEGIN
> +DECLARE a int;
> +-- SQL statement inside
> +SELECT 1 INTO a;
> +RETURN a;
> +END $
> +SELECT COUNT(DISTINCT d) FROM t1, t2  WHERE a = c AND b = f1();
> +COUNT(DISTINCT d)
> +2
> +DROP FUNCTION f1;
> +DROP TABLE t1, t2;
>   # ------------------------------------------------------------------
>   # -- End of 5.1 tests
>   # ------------------------------------------------------------------
>
> === modified file 'mysql-test/t/sp.test'
> --- a/mysql-test/t/sp.test	2009-12-23 13:44:03 +0000
> +++ b/mysql-test/t/sp.test	2011-06-21 10:03:04 +0000
> @@ -8350,6 +8350,33 @@ SET @@GLOBAL.init_connect= @old_init_con
>   DROP PROCEDURE p2;
>   DROP PROCEDURE p5;
>
> +--echo #
> +--echo # Bug#11766594  59736: SELECT DISTINCT.. INCORRECT RESULT WITH DETERMINISTIC
> FUNCTION IN WHERE C
> +--echo #
> +
> +CREATE TABLE t1 (a INT, b INT, KEY(b));
> +CREATE TABLE t2 (c INT, d INT, KEY(c));
> +INSERT INTO t1 VALUES (1,1),(1,1),(1,2);
> +INSERT INTO t2 VALUES (1,1),(1,2);
> +
> +DELIMITER $;
> +
> +CREATE FUNCTION f1() RETURNS INT DETERMINISTIC
> +BEGIN
> +  DECLARE a int;
> +  -- SQL statement inside
> +  SELECT 1 INTO a;
> +  RETURN a;
> +END $
> +
> +DELIMITER ;$
> +
> +SELECT COUNT(DISTINCT d) FROM t1, t2  WHERE a = c AND b = f1();
> +
> +DROP FUNCTION f1;
> +DROP TABLE t1, t2;
> +
> +
>   --echo # ------------------------------------------------------------------
>   --echo # -- End of 5.1 tests
>   --echo # ------------------------------------------------------------------
>
> === modified file 'sql/sql_base.cc'
> --- a/sql/sql_base.cc	2011-05-16 20:04:01 +0000
> +++ b/sql/sql_base.cc	2011-06-21 10:03:04 +0000
> @@ -7576,7 +7576,7 @@ bool setup_fields(THD *thd, Item **ref_p
>       if (item->with_sum_func&&  item->type() !=
> Item::SUM_FUNC_ITEM&&
>   	sum_func_list)
>         item->split_sum_func(thd, ref_pointer_array, *sum_func_list);
> -    thd->used_tables|= item->used_tables();
> +    thd->lex->used_tables|= item->used_tables();
>       thd->lex->current_select->cur_pos_in_select_list++;
>     }
>     thd->lex->current_select->is_item_list_lookup=
> save_is_item_list_lookup;
> @@ -7923,7 +7923,7 @@ insert_fields(THD *thd, Name_resolution_
>         views and natural joins this update is performed inside the loop below.
>       */
>       if (table)
> -      thd->used_tables|= table->map;
> +      thd->lex->used_tables|= table->map;
>
>       /*
>         Initialize a generic field iterator for the current table reference.
> @@ -8008,7 +8008,7 @@ insert_fields(THD *thd, Name_resolution_
>             field_table= nj_col->table_ref->table;
>             if (field_table)
>             {
> -            thd->used_tables|= field_table->map;
> +            thd->lex->used_tables|= field_table->map;
>               field_table->covering_keys.intersect(field->part_of_key);
>               field_table->merge_keys.merge(field->part_of_key);
>               field_table->used_fields++;
> @@ -8016,7 +8016,7 @@ insert_fields(THD *thd, Name_resolution_
>           }
>         }
>         else
> -        thd->used_tables|= item->used_tables();
> +        thd->lex->used_tables|= item->used_tables();
>         thd->lex->current_select->cur_pos_in_select_list++;
>       }
>       /*
>
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc	2011-05-21 08:43:54 +0000
> +++ b/sql/sql_class.cc	2011-06-21 10:03:04 +0000
> @@ -647,7 +647,6 @@ THD::THD()
>     is_slave_error= thread_specific_used= FALSE;
>     hash_clear(&handler_tables_hash);
>     tmp_table=0;
> -  used_tables=0;
>     cuted_fields= sent_row_count= row_count= 0L;
>     limit_found_rows= 0;
>     row_count_func= -1;
>
> === modified file 'sql/sql_class.h'
> --- a/sql/sql_class.h	2011-05-18 14:40:01 +0000
> +++ b/sql/sql_class.h	2011-06-21 10:03:04 +0000
> @@ -1732,13 +1732,6 @@ public:
>     */
>     ha_rows    examined_row_count;
>
> -  /*
> -    The set of those tables whose fields are referenced in all subqueries
> -    of the query.
> -    TODO: possibly this it is incorrect to have used tables in THD because
> -    with more than one subquery, it is not clear what does the field mean.
> -  */
> -  table_map  used_tables;
>     USER_CONN *user_connect;
>     CHARSET_INFO *db_charset;
>     /*
>
> === modified file 'sql/sql_insert.cc'
> --- a/sql/sql_insert.cc	2011-05-16 20:04:01 +0000
> +++ b/sql/sql_insert.cc	2011-06-21 10:03:04 +0000
> @@ -629,7 +629,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t
>     lock_type= table_list->lock_type;
>
>     thd_proc_info(thd, "init");
> -  thd->used_tables=0;
> +  thd->lex->used_tables=0;
>     values= its++;
>     value_count= values->elements;
>
> @@ -777,7 +777,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t
>       }
>       else
>       {
> -      if (thd->used_tables)			// Column used in values()
> +      if (thd->lex->used_tables)		 // Column used in values()
>   	restore_record(table,s->default_values);	// Get empty record
>         else
>         {
>
> === modified file 'sql/sql_lex.cc'
> --- a/sql/sql_lex.cc	2011-02-22 21:03:32 +0000
> +++ b/sql/sql_lex.cc	2011-06-21 10:03:04 +0000
> @@ -358,6 +358,7 @@ void lex_start(THD *thd)
>     lex->server_options.port= -1;
>
>     lex->is_lex_started= TRUE;
> +  lex->used_tables= 0;
>     DBUG_VOID_RETURN;
>   }
>
>
> === modified file 'sql/sql_lex.h'
> --- a/sql/sql_lex.h	2011-02-22 21:03:32 +0000
> +++ b/sql/sql_lex.h	2011-06-21 10:03:04 +0000
> @@ -1828,6 +1828,14 @@ typedef struct st_lex : public Query_tab
>     uint create_select_pos;
>     bool create_select_in_comment;
>
> +  /*
> +    The set of those tables whose fields are referenced in all subqueries
> +    of the query.
> +    TODO: possibly this it is incorrect to have used tables in THD because
> +    with more than one subquery, it is not clear what does the field mean.
> +  */
> +  table_map  used_tables;
> +
>     st_lex();
>
>     virtual ~st_lex()
>
> === modified file 'sql/sql_prepare.cc'
> --- a/sql/sql_prepare.cc	2011-05-16 20:04:01 +0000
> +++ b/sql/sql_prepare.cc	2011-06-21 10:03:04 +0000
> @@ -1382,7 +1382,7 @@ static int mysql_test_select(Prepared_st
>     if (open_normal_and_derived_tables(thd, tables, 0))
>       goto error;
>
> -  thd->used_tables= 0;                        // Updated by setup_fields
> +  thd->lex->used_tables= 0;                        // Updated by setup_fields
>
>     /*
>       JOIN::prepare calls
> @@ -1551,7 +1551,7 @@ static bool select_like_stmt_test(Prepar
>     if (specific_prepare&&  (*specific_prepare)(thd))
>       DBUG_RETURN(TRUE);
>
> -  thd->used_tables= 0;                        // Updated by setup_fields
> +  thd->lex->used_tables= 0;                        // Updated by setup_fields
>
>     /* Calls JOIN::prepare */
>     DBUG_RETURN(lex->unit.prepare(thd, 0, setup_tables_done_option));
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2011-04-22 07:20:55 +0000
> +++ b/sql/sql_select.cc	2011-06-21 10:03:04 +0000
> @@ -406,7 +406,7 @@ fix_inner_refs(THD *thd, List<Item>  &all
>
>       if (!ref->fixed&&  ref->fix_fields(thd, 0))
>         return TRUE;
> -    thd->used_tables|= item->used_tables();
> +    thd->lex->used_tables|= item->used_tables();
>     }
>     return false;
>   }
> @@ -1632,7 +1632,7 @@ JOIN::optimize()
>
>       if (exec_tmp_table1->distinct)
>       {
> -      table_map used_tables= thd->used_tables;
> +      table_map used_tables= thd->lex->used_tables;
>         JOIN_TAB *last_join_tab= join_tab+tables-1;
>         do
>         {
> @@ -2526,7 +2526,7 @@ mysql_select(THD *thd, Item ***rref_poin
>       if (!(join= new JOIN(thd, fields, select_options, result)))
>   	DBUG_RETURN(TRUE);
>       thd_proc_info(thd, "init");
> -    thd->used_tables=0;                         // Updated by setup_fields
> +    thd->lex->used_tables=0;                         // Updated by
> setup_fields
>       err= join->prepare(rref_pointer_array, tables, wild_num,
>                          conds, og_num, order, group, having, proc_param,
>                          select_lex, unit);
> @@ -16949,7 +16949,7 @@ static void select_describe(JOIN *join,
>   	  need_order=0;
>   	  extra.append(STRING_WITH_LEN("; Using filesort"));
>   	}
> -	if (distinct&  test_all_bits(used_tables,thd->used_tables))
> +	if (distinct&  test_all_bits(used_tables,thd->lex->used_tables))
>   	  extra.append(STRING_WITH_LEN("; Distinct"));
>
>           for (uint part= 0; part<  tab->ref.key_parts; part++)
>
>
>
>


Thread
bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Sergey Glukhov21 Jun
  • Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Øystein Grøvlen23 Jun
  • Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Roy Lyseng5 Jul
    • Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Sergey Glukhov7 Jul
      • Re: bzr commit into mysql-5.1 branch (sergey.glukhov:3649) Bug#11766594Roy Lyseng7 Jul