← Back to team overview

maria-developers team mailing list archive

Re: [Commits] Rev 3770: MDEV-4042 & MDEV-4536 fix. in file:///home/bell/maria/bzr/work-maria-5.5-MDEV-4536/

 

Hi Sanja,

The patch doesn't take one thing into account. Consider a query:

explain 
select * from t12 
where 
  b>3 or exists (select max(t10.b) Z 
                 from t10 
                 where t10.a in ( select t11.a from t11) 
                 group by t10.c 
                 having Z<100);

+--+------------+-----------+------+-------------+------------+-------+----+----+-------------------------------+
|id|select_type |table      |type  |possible_keys|key         |key_len|ref |rows|Extra                          |
+--+------------+-----------+------+-------------+------------+-------+----+----+-------------------------------+
|1 |PRIMARY     |t12        |ALL   |NULL         |NULL        |NULL   |NULL|10  |                               |
|2 |SUBQUERY    |t10        |ALL   |NULL         |NULL        |NULL   |NULL|6   |Using temporary; Using filesort|
|2 |SUBQUERY    |<subquery3>|eq_ref|distinct_key |distinct_key|4      |func|1   |                               |
|3 |MATERIALIZED|t11        |ALL   |NULL         |NULL        |NULL   |NULL|6   |                               |
+--+------------+-----------+------+-------------+------------+-------+----+----+-------------------------------+

Here, the EXISTS subquery is the one that will have the problem.
I have added "in ( select t11.a from t11)" so that EXISTS subquery's join_tab array is not linear.

The problem is not visible to the user, but it can be seen in debugger.
Put a breakpoint in JOIN::cleanup(), with these commands:

  p table->alias->Ptr
  p table->file

Run the query and you'll see:

  Breakpoint 5, st_join_table::cleanup (this=0x7fff280dbea0) at /home/psergey/dev2/5.5/sql/sql_select.cc:10248
  $30 = 0x7fff28000af0 "t12"
  $31 = (ha_innobase *) 0x7fff28047198
...
  
  Breakpoint 5, st_join_table::cleanup (this=0x7fff28053078) at /home/psergey/dev2/5.5/sql/sql_select.cc:10248
  $32 = 0xd05418 ""
  $33 = (ha_heap *) 0x7fff2805ab98
...

  Breakpoint 5, st_join_table::cleanup (this=0x7fff28051ef8) at /home/psergey/dev2/5.5/sql/sql_select.cc:10248
  $36 = 0x7fff28025720 "t10"
  $37 = (ha_innobase *) 0x7fff28009558
...
  
  Breakpoint 5, st_join_table::cleanup (this=0x7fff28052218) at /home/psergey/dev2/5.5/sql/sql_select.cc:10248
  $38 = 0xd30a69 "sj-materialize"
  $39 = (ha_heap *) 0x7fff28054398
...

Note that in subquery join_tab->cleanup() was called for t10, and for sj-materialize. For t11 (which is inside 
the sj-materialize) it was not called.

This caused by this loop being flat loop:

> @@ -10646,6 +10646,12 @@ void JOIN::cleanup(bool full)
>        {
>  	tab->cleanup();
>        }
> +      if (join_tab != table_access_tabs)
> +      {
> +        for (uint i= 0; i < top_table_access_tabs_count; i++)a
              ^^^^^^^^^^^ here.
> +          table_access_tabs[i].cleanup();
> +        table_access_tabs= join_tab; // avoid second cleanup()
> +      }
>        cleaned= true;
>      }
>      else
 
It turns out, I was incorrect with the statement that "table_access_tabs array is always flat".

On Tue, May 21, 2013 at 08:30:35AM +0300, sanja@xxxxxxxxxxxxxxxx wrote:
> At file:///home/bell/maria/bzr/work-maria-5.5-MDEV-4536/
> 
> ------------------------------------------------------------
> revno: 3770
> revision-id: sanja@xxxxxxxxxxxxxxxx-20130521053031-xe34093yhvey0l5o
> parent: monty@xxxxxxxxxxxx-20130515132812-pnjhza9iugz89356
> committer: sanja@xxxxxxxxxxxxxxxx
> branch nick: work-maria-5.5-MDEV-4536
> timestamp: Tue 2013-05-21 08:30:31 +0300
> message:
>   MDEV-4042 & MDEV-4536 fix.
>   
>   Optimization JOIN tabs also should be cleaned up (in case of EXPLAIN when we cleaning up everythibng at the end of execution).

> === added file 'mysql-test/r/innodb_explain.result'
> --- a/mysql-test/r/innodb_explain.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/r/innodb_explain.result	2013-05-21 05:30:31 +0000
> @@ -0,0 +1,34 @@
> +drop table if exists t1,t2,t3;
> +#
> +# MDEV-4536: sql_base.cc:1598: bool close_thread_table(THD*, TABLE**):
> +#  Assertion `table->key_read == 0' failed.
> +#
> +set optimizer_switch="index_merge=on,index_merge_union=on,index_merge_sort_union=on,index_merge_intersection=on,index_merge_sort_intersection=off,engine_condition_pushdown=off,index_condition_pushdown=on,derived_merge=on,derived_with_keys=on,firstmatch=on,loosescan=on,materialization=on,in_to_exists=on,semijoin=on,partial_match_rowid_merge=on,partial_match_table_scan=on,subquery_cache=on,mrr=off,mrr_cost_based=off,mrr_sort_keys=off,outer_join_with_cache=on,semijoin_with_cache=on,join_cache_incremental=on,join_cache_hashed=on,join_cache_bka=on,optimize_join_buffer_size=off,table_elimination=on,extended_keys=off";
> +CREATE TABLE t1 (pk int auto_increment primary key, `col_int_key` int(11),
> +key `col_int_key` (`col_int_key`),`col_varchar_key` varchar(128), key
> +(col_varchar_key)) engine=innodb;
> +EXPLAIN SELECT 1 FROM t1 AS alias1 WHERE EXISTS ( SELECT SQ2_alias1 . col_int_key AS SQ2_field1 FROM ( t1 AS SQ2_alias1 RIGHT OUTER JOIN t1 AS SQ2_alias2 ON (SQ2_alias2 . col_int_key = SQ2_alias1 . col_int_key ) ) GROUP BY SQ2_field1 HAVING SQ2_alias1 . col_int_key >= 7 );
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	PRIMARY	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible WHERE
> +2	SUBQUERY	SQ2_alias2	index	NULL	col_int_key	5	NULL	1	Using index; Using temporary; Using filesort
> +2	SUBQUERY	SQ2_alias1	ref	col_int_key	col_int_key	5	test.SQ2_alias2.col_int_key	1	Using where; Using index
> +set optimizer_switch=default;
> +drop table t1;
> +#
> +# MDEV-4042: Assertion `table->key_read == 0' fails in
> +# close_thread_table on EXPLAIN with GROUP BY and HAVING in
> +# EXISTS SQ, InnoDB
> +#
> +CREATE TABLE t1 (a INT) ENGINE=InnoDB;
> +CREATE TABLE t2 (b INT PRIMARY KEY, c INT) ENGINE=InnoDB;
> +CREATE TABLE t3 (d INT) ENGINE=InnoDB;
> +EXPLAIN 
> +SELECT * FROM t1
> +WHERE EXISTS ( SELECT b FROM t2, t3
> +GROUP BY b
> +HAVING b != 3 );
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	PRIMARY	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible WHERE
> +2	SUBQUERY	t2	index	NULL	PRIMARY	4	NULL	1	Using index; Using temporary; Using filesort
> +2	SUBQUERY	t3	ALL	NULL	NULL	NULL	NULL	1	Using join buffer (flat, BNL join)
> +drop table t1,t2,t3;
> 
> === added file 'mysql-test/t/innodb_explain.test'
> --- a/mysql-test/t/innodb_explain.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/innodb_explain.test	2013-05-21 05:30:31 +0000
> @@ -0,0 +1,41 @@
> +#
> +# Innodb specific explains
> +
> +--disable_warnings
> +drop table if exists t1,t2,t3;
> +--enable_warnings
> +
> +--source include/have_innodb.inc
> +
> +--echo #
> +--echo # MDEV-4536: sql_base.cc:1598: bool close_thread_table(THD*, TABLE**):
> +--echo #  Assertion `table->key_read == 0' failed.
> +--echo #
> +set optimizer_switch="index_merge=on,index_merge_union=on,index_merge_sort_union=on,index_merge_intersection=on,index_merge_sort_intersection=off,engine_condition_pushdown=off,index_condition_pushdown=on,derived_merge=on,derived_with_keys=on,firstmatch=on,loosescan=on,materialization=on,in_to_exists=on,semijoin=on,partial_match_rowid_merge=on,partial_match_table_scan=on,subquery_cache=on,mrr=off,mrr_cost_based=off,mrr_sort_keys=off,outer_join_with_cache=on,semijoin_with_cache=on,join_cache_incremental=on,join_cache_hashed=on,join_cache_bka=on,optimize_join_buffer_size=off,table_elimination=on,extended_keys=off";
> +
> +CREATE TABLE t1 (pk int auto_increment primary key, `col_int_key` int(11),
> +key `col_int_key` (`col_int_key`),`col_varchar_key` varchar(128), key
> +(col_varchar_key)) engine=innodb;
> +
> +EXPLAIN SELECT 1 FROM t1 AS alias1 WHERE EXISTS ( SELECT SQ2_alias1 . col_int_key AS SQ2_field1 FROM ( t1 AS SQ2_alias1 RIGHT OUTER JOIN t1 AS SQ2_alias2 ON (SQ2_alias2 . col_int_key = SQ2_alias1 . col_int_key ) ) GROUP BY SQ2_field1 HAVING SQ2_alias1 . col_int_key >= 7 );
> +
> +
> +set optimizer_switch=default;
> +drop table t1;
> +
> +--echo #
> +--echo # MDEV-4042: Assertion `table->key_read == 0' fails in
> +--echo # close_thread_table on EXPLAIN with GROUP BY and HAVING in
> +--echo # EXISTS SQ, InnoDB
> +--echo #
> +CREATE TABLE t1 (a INT) ENGINE=InnoDB;
> +CREATE TABLE t2 (b INT PRIMARY KEY, c INT) ENGINE=InnoDB;
> +CREATE TABLE t3 (d INT) ENGINE=InnoDB;
> +
> +EXPLAIN 
> +SELECT * FROM t1
> +WHERE EXISTS ( SELECT b FROM t2, t3
> +GROUP BY b
> +HAVING b != 3 );
> +
> +drop table t1,t2,t3;
> 
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2013-05-07 11:05:09 +0000
> +++ b/sql/sql_select.cc	2013-05-21 05:30:31 +0000
> @@ -3680,8 +3680,8 @@ make_join_statistics(JOIN *join, List<TA
>    if (pull_out_semijoin_tables(join))
>      DBUG_RETURN(TRUE);
>  
> -  join->join_tab=stat;
> -  join->top_join_tab_count= table_count;
> +  join->table_access_tabs= join->join_tab=stat;
> +  join->top_table_access_tabs_count= join->top_join_tab_count= table_count;
>    join->map2table=stat_ref;
>    join->table= table_vector;
>    join->const_tables=const_count;
> @@ -10646,6 +10646,12 @@ void JOIN::cleanup(bool full)
>        {
>  	tab->cleanup();
>        }
> +      if (join_tab != table_access_tabs)
> +      {
> +        for (uint i= 0; i < top_table_access_tabs_count; i++)
> +          table_access_tabs[i].cleanup();
> +        table_access_tabs= join_tab; // avoid second cleanup()
> +      }
>        cleaned= true;
>      }
>      else
> 

> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

-- 
BR
 Sergei
-- 
Sergei Petrunia, Software Developer
Monty Program AB, http://askmonty.org
Blog: http://s.petrunia.net/blog