Hello Jorgen,
Jorgen Loland a écrit, Le 25.03.2011 14:10:
> #At file:///export/home/jl208045/mysql/mysql-trunk-59415/ based on
> revid:olav.sandstaa@stripped
>
> 3323 Jorgen Loland 2011-03-25
> BUG#11766327: Range analysis should not be done many times
> for the same index
>
> (formerly 59415)
The patch looks good, only minor details below. Quite close to a push.
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2011-03-24 13:16:36 +0000
> +++ b/sql/sql_select.cc 2011-03-25 13:10:57 +0000
> @@ -3240,8 +3240,8 @@ JOIN::exec()
> DBUG_VOID_RETURN;
> curr_table->select->cond->fix_fields(thd, 0);
> }
> - curr_table->set_select_cond(curr_table->select->cond, __LINE__);
> - curr_table->select_cond->top_level_item();
> + curr_table->set_cond(curr_table->select->cond, __LINE__);
> + curr_table->get_cond()->top_level_item();
> DBUG_EXECUTE("where",print_where(curr_table->select->cond,
> "select and having",
> QT_ORDINARY););
> @@ -9244,28 +9244,39 @@ JOIN::make_simple_join(JOIN *parent, TAB
> DBUG_RETURN(FALSE);
> }
>
> +/**
> + Extend e1 by AND'ing e2 to the condition e1 points to.
> +
> + @param[in,out] e1 Pointer to condition that will be extended with e2
> + @param e2 Condition that will extend e1
>
> -inline void add_cond_and_fix(Item **e1, Item *e2)
> + @retval true if there was a memory allocation error, in which case
> + e1 remains unchanged
> + @retval false otherwise
> +*/
> +inline bool add_cond_and_fix(Item **e1, Item *e2)
now it returns bool but there are a couple of calls to this function
which ignore the new return code.
> {
> if (*e1)
> {
> if (!e2)
> - return;
> - Item *res;
> - if ((res= new Item_cond_and(*e1, e2)))
> - {
> - *e1= res;
> - res->quick_fix_field();
> - res->update_used_tables();
> - }
> + return false;
> + Item *res= new Item_cond_and(*e1, e2);
> + if (!res)
could add unlikely()
> + return true;
> +
> + *e1= res;
> + res->quick_fix_field();
> + res->update_used_tables();
> +
> }
> else
> *e1= e2;
> + return false;
> }
>
> @@ -9355,9 +9366,7 @@ static void add_not_null_conds(JOIN *joi
> DBUG_EXECUTE("where",print_where(notnull,
> referred_tab->table->alias,
> QT_ORDINARY););
> - Item *new_cond= referred_tab->select_cond;
> - add_cond_and_fix(&new_cond, notnull);
> - referred_tab->set_select_cond(new_cond, __LINE__);
> + referred_tab->extend_and_fix_cond(notnull, __LINE__);
Return code is not checked.
Note: if the code already had this problem before the patch, then ignore
my remarks.
> @@ -9541,9 +9532,8 @@ static bool pushdown_on_conditions(JOIN*
> tmp_cond= new Item_func_trig_cond(tmp_cond,
> &cond_tab->not_null_compl);
> if (!tmp_cond)
> DBUG_RETURN(1);
> - tmp_cond->quick_fix_field();
You sure this line is not needed ?
>
> - if (extend_select_cond(cond_tab, tmp_cond))
> + if (cond_tab->extend_and_fix_jt_and_sel_cond(tmp_cond, __LINE__))
> DBUG_RETURN(1);
> }
> }
> @@ -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);
> 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()
> + return true;
> +
> + if (select)
> + {
> + DBUG_PRINT("info",
> + ("select::cond extended. Change %p -> %p "
> + "at line %u tab %p select %p",
> + select->cond, cond, line, this, select));
> + select->cond= cond;
> + }
> + return false;
> +}
> +
> +/**
> + Extend JOIN_TAB->cond by AND'ing add_cond to it
> +
> + @param add_cond The condition to AND with the existing cond
> + for this JOIN_TAB
> + @param line Code line this method was called from
> +
> + @retval true if there was a memory allocation error, in which case
> + JOIN_TAB::cond remains unchanged
> + @retval false otherwise
> +*/
> +bool JOIN_TAB::extend_and_fix_cond(Item *add_cond, uint line)
> +{
> + Item *old_cond= cond;
> + if (add_cond_and_fix(&cond, add_cond))
could add unlikely()
> + return true;
> + DBUG_PRINT("info", ("JOIN_TAB::cond extended. Change %p -> %p "
> + "at line %u tab %p",
> + old_cond, cond, line, this));
> + return false;
> +}
> +
> +
> +
>