Roy Lyseng a écrit, Le 12.08.2010 14:15:
> #At file:///home/rl136806/mysql/repo/mysql-work5/ based on
> revid:guilhem.bichot@stripped
>
> 3228 Roy Lyseng 2010-08-12
> Bug#50489: another segfault in fix_semijoin_strategies...
> @@ -3250,6 +3248,12 @@ JOIN::destroy()
> if (exec_tmp_table2)
> free_tmp_table(thd, exec_tmp_table2);
> destroy_sj_tmp_tables(this);
> + List_iterator<TABLE_LIST> sj_list_it(select_lex->sj_nests);
> + TABLE_LIST *sj_nest;
> + while ((sj_nest= sj_list_it++))
> + {
{ and } could optionally be removed.
> + sj_nest->sj_mat_info= NULL;
> + }
seeing the destroy_sj_tmp_tables() above, I think you chose a good place.
> @@ -4890,14 +4894,16 @@ static bool optimize_semijoin_nests(JOIN
> DBUG_ENTER("optimize_semijoin_nests");
> List_iterator<TABLE_LIST> sj_list_it(join->select_lex->sj_nests);
> TABLE_LIST *sj_nest;
> - /*
> - The statement may have been executed with 'semijoin=on' earlier.
> - We need to verify that 'semijoin=on' still holds.
> - */
> - if (join->thd->optimizer_switch_flag(OPTIMIZER_SWITCH_SEMIJOIN) &&
> - join->thd->optimizer_switch_flag(OPTIMIZER_SWITCH_MATERIALIZATION))
> +
> + while ((sj_nest= sj_list_it++))
> {
> - while ((sj_nest= sj_list_it++))
> + /* As a precaution, reset pointers that were used in prior execution */
> + sj_nest->sj_mat_info= NULL;
> + sj_nest->nested_join->sjm.positions= NULL;
ok, or just DBUG_ASSERT that they are already NULL as they should be.
My humble opinion is that if we keep the reset at start like above,
resetting at end of the prior execution is superfluous, it just wastes
CPU time and lines of code. I would have preffered only resets at end of
the prior execution.
But other people have different opinions about such "defensive
programming", and I wasted your time enough on this micro-topic, so do
as you prefer :-)
> @@ -8413,6 +8433,13 @@ static void fix_semijoin_strategies_for_
> pos->sj_strategy= SJ_OPT_NONE;
> remaining_tables |= s->table->map;
> }
> +
> + List_iterator<TABLE_LIST> sj_list_it(join->select_lex->sj_nests);
> + TABLE_LIST *sj_nest;
> + while ((sj_nest= sj_list_it++))
> + sj_nest->nested_join->sjm.positions= NULL;
optionally there could be a comment stating that this sjm or positions
are not needed anymore.
Ok to push.
