On 31.03.11 19.06, Guilhem Bichot wrote:
>> @@ -11072,10 +11056,12 @@ bool setup_sj_materialization(JOIN_TAB *
>> */
>> for (i= 0; i < sjm->table_count; i++)
>> {
>> - remove_sj_conds(&tab[i].select_cond);
>> + Item *cond= tab[i].get_cond();
>
> If this intermediate variable is not needed I suggest to delete it.
> But maybe it is needed, I haven't thought about return-by-value, lvalue etc.
>
>> + remove_sj_conds(&cond);
This does not seem correct to me. remove_sj_conds() is supposed to modify
tab[i].select_cond, but after the fix it modifies the local copy.
I also have a personal preference issue: get_ as a prefix is redundant, I would
replace get_cond() with condition() (but keep set_condition() for the setter).
>> if (tab[i].select)
>> remove_sj_conds(&tab[i].select->cond);
>> }
>> +
>> if (!(sjm->in_equality= create_subquery_equalities(thd, emb_sj_nest)))
>> DBUG_RETURN(TRUE); /* purecov: inspected */
>> }
>> @@ -11464,6 +11450,58 @@ uint JOIN_TAB::get_sj_strategy() const
>> return s;
>> }
>>
>> +/**
>> + Extend JOIN_TAB->cond and JOIN_TAB->select->cond by AND'ing
>> + add_cond to them
>> +
>> + @param add_cond The condition to AND with the existing conditions
>> + @param line Code line this method was called from
>> +
>> + @retval true if there was a memory allocation error, in which + case the
>> conditions remain unchanged
>> + @retval false otherwise
>> +
>> +*/
>> +bool JOIN_TAB::extend_and_fix_jt_and_sel_cond(Item *add_cond, uint line)
>> +{
>> + if (extend_and_fix_cond(add_cond, line))
>
> could add unlikely()
I'm a bit afraid of the "code contamination" of these unlikely() macros. There
is also an option to gather branch prediction statistics using two-stage
compilations. (The first stage gathers statistics, the second uses statistics to
apply branch prediction hints). The benefit of this is that code need no
changes, the drawback is the need for two-stage compilation, which makes it hard
to do in developer sandboxes.
Thanks,
Roy
