Hi,
I am not convinced that this fix is safe. If you skip assigning pos to
loose_scan_pos, I think you will be returning a cost that is based on
something else than what is asked for by the calling function. Hence, it
will assume that loose_scan can be performed with a certain cost when
the actual case is probably that loose_scan can not be used at all. In
the actual test case in subquery_mat_all this does not matter since
another strategy is picked anyway, but can we be sure that is always the
case?
I think you at least should ensure that DBL_MAX is returned as the total
cost from optimizer_wo_join_buffering() in this case. That should
prevent loose_scan from being picked as the best strategy for the given
join order.
--
Øystein
On 07/20/10 03:29 PM, Tor Didriksen wrote:
> #At file:///export/home/didrik/repo/next-mr-opt-backporting-valgrind/ based on
> revid:roy.lyseng@stripped
>
> 3217 Tor Didriksen 2010-07-20
> Bug #55042 valgrind reports error in best_access_path when running
> subquery_mat_all
>
> The code was using loose_scan_pos, even though it was explicitly set to
> invalid.
> @ sql/sql_select.cc
> Do not use loose_scan_pos in optimize_wo_join_buffering() if it has a cost
> of DBL_MAX
> (as determined by best_access_path())
>
> modified:
> sql/sql_select.cc
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2010-07-16 12:21:31 +0000
> +++ b/sql/sql_select.cc 2010-07-20 13:28:59 +0000
> @@ -13320,20 +13320,17 @@ void optimize_wo_join_buffering(JOIN *jo
> double *reopt_rec_count, double *reopt_cost,
> double *sj_inner_fanout)
> {
> - double cost, outer_fanout, inner_fanout= 1.0;
> + double cost=0.0, outer_fanout=1.0, inner_fanout= 1.0;
> table_map reopt_remaining_tables= last_remaining_tables;
> uint i;
>
> + DBUG_ENTER("optimize_wo_join_buffering");
> +
> if (first_tab> join->const_tables)
> {
> cost= join->positions[first_tab - 1].prefix_cost.total_cost();
> outer_fanout= join->positions[first_tab - 1].prefix_record_count;
> }
> - else
> - {
> - cost= 0.0;
> - outer_fanout= 1;
> - }
>
> for (i= first_tab; i<= last_tab; i++)
> reopt_remaining_tables |= join->positions[i].table->table->map;
> @@ -13346,14 +13343,19 @@ void optimize_wo_join_buffering(JOIN *jo
> if ((i == first_tab&& first_alt) ||
> join->positions[i].use_join_buffer)
> {
> /* Find the best access method that would not use join buffering */
> - best_access_path(join, rs, reopt_remaining_tables, i,
> - test(i< no_jbuf_before), inner_fanout*outer_fanout,
> -&pos,&loose_scan_pos);
> + best_access_path(join,
> + rs,
> + reopt_remaining_tables,
> + i,
> + test(i< no_jbuf_before),
> + inner_fanout*outer_fanout,
> +&pos,
> +&loose_scan_pos);
> }
> else
> pos= join->positions[i];
>
> - if ((i == first_tab&& first_alt))
> + if ((i == first_tab&& first_alt&& loose_scan_pos.read_time<
> DBL_MAX))
> pos= loose_scan_pos;
>
> reopt_remaining_tables&= ~rs->table->map;
> @@ -13369,6 +13371,7 @@ void optimize_wo_join_buffering(JOIN *jo
> *reopt_rec_count= outer_fanout;
> *reopt_cost= cost;
> *sj_inner_fanout= inner_fanout;
> + DBUG_VOID_RETURN;
> }
>
>
>
>
>
>
>
--
Øystein Grøvlen, Senior Staff Engineer
Sun Microsystems, Database Group
Trondheim, Norway
