List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:December 1 2010 10:54am
Subject:Re: bzr commit into mysql-5.1 branch (ole.john.aske:3474) Bug#58134
View as plain text  
Hi Ole John,

The patch looks correct. OK to push.

One minor details I noticed (not directly related to your patch but 
still) is that in mysql-trunk (and in the optimizer trees) the if- test 
in front of the call to make_cond_for_table() is extended with an extra 
test for !first_inner_tab:

       if 
(thd->optimizer_switch_flag(OPTIMIZER_SWITCH_ENGINE_CONDITION_PUSHDOWN) &&
               !first_inner_tab)
           {
             Item *push_cond=
               make_cond_for_table(tmp, tab->table->map, 
tab->table->map, 0);

I do not think that relates to your fix but it still will probably 
influence on when and what get pushed down to NDB?

Olav


On 19/11/2010 11:09, Ole John Aske wrote:
> #At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-5.1/ based on
> revid:ole.john.aske@stripped
>
>   3474 Ole John Aske	2010-11-19
>        Fix for bug#58134, Incorrectly condition pushdown inside subquery to NDB
> engine.
>
>        An incorrect 'table_map' containing both the table itself,
>        and possible any outer-refs if this was the last table in
>        the subquery, was presented to make_cond_for_table().
>
>        As a pushed condition is only able to refer column from the table
>        the condition is pushed to, nothing else than columns from the
>        table itself (tab->table->map) may be refered in the pushed condition
>        constructed by 'push_cond= make_cond_for_table()'.
>
>        Also fix a minor 'copy and paste' bug in a comment
>        inside make_cond_for_table().
>
>      modified:
>        mysql-test/suite/ndb/r/ndb_condition_pushdown.result
>        mysql-test/suite/ndb/t/ndb_condition_pushdown.test
>        sql/sql_select.cc
> === modified file 'mysql-test/suite/ndb/r/ndb_condition_pushdown.result'
> --- a/mysql-test/suite/ndb/r/ndb_condition_pushdown.result	2008-01-31 13:47:50 +0000
> +++ b/mysql-test/suite/ndb/r/ndb_condition_pushdown.result	2010-11-19 10:09:13 +0000
> @@ -1916,5 +1916,37 @@ a	b	d
>   50	5	21845
>   Warnings:
>   Warning	4294	Scan filter is too large, discarded
> +set engine_condition_pushdown = on;
> +create table t (pk int, i int) engine = ndb;
> +insert into t values (1,3), (3,6), (6,9), (9,1);
> +create table subq (pk int, i int) engine = ndb;
> +insert into subq values (1,3), (3,6), (6,9), (9,1);
> +explain extended
> +select * from t where exists
> +(select * from t as subq where subq.i=3 and t.i=3);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
> +1	PRIMARY	t	ALL	NULL	NULL	NULL	NULL	4	50.00	Using where
> +2	DEPENDENT SUBQUERY	subq	ALL	NULL	NULL	NULL	NULL	4	100.00	Using where with pushed
> condition: (`test`.`subq`.`i` = 3)
> +Warnings:
> +Note	1276	Field or reference 'test.t.i' of SELECT #2 was resolved in SELECT #1
> +Note	1003	select `test`.`t`.`pk` AS `pk`,`test`.`t`.`i` AS `i` from `test`.`t` where
> exists(select 1 from `test`.`t` `subq` where ((`test`.`subq`.`i` = 3) and (`test`.`t`.`i`
> = 3)))
> +explain extended
> +select * from t where exists
> +(select * from subq where subq.i=3 and t.i=3);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
> +1	PRIMARY	t	ALL	NULL	NULL	NULL	NULL	4	100.00	Using where
> +2	DEPENDENT SUBQUERY	subq	ALL	NULL	NULL	NULL	NULL	4	50.00	Using where with pushed
> condition: (`test`.`subq`.`i` = 3)
> +Warnings:
> +Note	1276	Field or reference 'test.t.i' of SELECT #2 was resolved in SELECT #1
> +Note	1003	select `test`.`t`.`pk` AS `pk`,`test`.`t`.`i` AS `i` from `test`.`t` where
> exists(select 1 from `test`.`subq` where ((`test`.`subq`.`i` = 3) and (`test`.`t`.`i` =
> 3)))
> +select * from t where exists
> +(select * from t as subq where subq.i=3 and t.i=3);
> +pk	i
> +1	3
> +select * from t where exists
> +(select * from subq where subq.i=3 and t.i=3);
> +pk	i
> +1	3
> +drop table t,subq;
>   set engine_condition_pushdown = @old_ecpd;
>   DROP TABLE t1,t2,t3,t4,t5;
>
> === modified file 'mysql-test/suite/ndb/t/ndb_condition_pushdown.test'
> --- a/mysql-test/suite/ndb/t/ndb_condition_pushdown.test	2008-01-31 13:47:50 +0000
> +++ b/mysql-test/suite/ndb/t/ndb_condition_pushdown.test	2010-11-19 10:09:13 +0000
> @@ -2050,5 +2050,31 @@ select a,b,d from t1
>       order by b;
>   --enable_query_log
>
> +# Bug#58134: Incorrectly condition pushdown inside subquery to NDB engine
> +set engine_condition_pushdown = on;
> +
> +create table t (pk int, i int) engine = ndb;
> +insert into t values (1,3), (3,6), (6,9), (9,1);
> +create table subq (pk int, i int) engine = ndb;
> +insert into subq values (1,3), (3,6), (6,9), (9,1);
> +
> +# 'Explain extended' to verify that only 'subq.i=3' is pushed
> +explain extended
> +select * from t where exists
> +  (select * from t as subq where subq.i=3 and t.i=3);
> +explain extended
> +  select * from t where exists
> +    (select * from subq where subq.i=3 and t.i=3);
> +
> +--sorted_result
> +select * from t where exists
> +  (select * from t as subq where subq.i=3 and t.i=3);
> +--sorted_result
> +select * from t where exists
> +  (select * from subq where subq.i=3 and t.i=3);
> +
> +drop table t,subq;
> +
> +
>   set engine_condition_pushdown = @old_ecpd;
>   DROP TABLE t1,t2,t3,t4,t5;
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2010-10-20 13:22:47 +0000
> +++ b/sql/sql_select.cc	2010-11-19 10:09:13 +0000
> @@ -6371,7 +6371,7 @@ make_join_select(JOIN *join,SQL_SELECT *
>   	  if (thd->variables.engine_condition_pushdown)
>             {
>               COND *push_cond=
> -              make_cond_for_table(tmp, current_map, current_map);
> +              make_cond_for_table(tmp, tab->table->map,
> tab->table->map);
>               if (push_cond)
>               {
>                 /* Push condition to handler */
> @@ -12857,7 +12857,7 @@ make_cond_for_table(COND *cond, table_ma
>   	new_cond->argument_list()->push_back(fix);
>         }
>         /*
> -	Item_cond_and do not need fix_fields for execution, its parameters
> +	Item_cond_or do not need fix_fields for execution, its parameters
>   	are fixed or do not need fix_fields, too
>         */
>         new_cond->quick_fix_field();
>
>    
>
>
>
>    


Thread
bzr commit into mysql-5.1 branch (ole.john.aske:3474) Bug#58134Ole John Aske19 Nov
  • Re: bzr commit into mysql-5.1 branch (ole.john.aske:3474) Bug#58134Roy Lyseng1 Dec
  • Re: bzr commit into mysql-5.1 branch (ole.john.aske:3474) Bug#58134Olav Sandstaa1 Dec