From: Roy Lyseng
Date: March 31 2011 6:30pm
Subject: Re: bzr commit into mysql-trunk branch (jorgen.loland:3323) Bug#11766327
List-Archive: http://lists.mysql.com/commits/134398
Message-Id: <4D94C83C.4050007@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

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