On 21.09.10 16.17, Guilhem Bichot wrote:
> Hello,
>
> Roy Lyseng a écrit, Le 21.09.2010 13:41:
>> #At file:///home/rl136806/mysql/repo/mysql-review/ based on
>> revid:roy.lyseng@stripped
>>
>> 3245 Roy Lyseng 2010-09-21
>> Refactoring: Make members positions and best_positions of class JOIN
>> dynamically allocated instead of fixed-size arrays.
>> This saves approx. 20 kbyte memory during optimization and execution
>> for normal-sized join operations.
>
> Good catch!
Anyone doing print sizeof(JOIN) would go huntin' for this one ;)
>
>> === modified file 'sql/sql_select.cc'
>> --- a/sql/sql_select.cc 2010-09-20 14:06:02 +0000
>> +++ b/sql/sql_select.cc 2010-09-21 11:36:51 +0000
>> @@ -4490,6 +4490,14 @@ make_join_statistics(JOIN *join, TABLE_L
>> if (!stat || !stat_ref || !table_vector)
>> DBUG_RETURN(1); // Eom /* purecov: inspected */
>>
>> + if (!(join->positions=
>> + (POSITION *)join->thd->alloc(sizeof(POSITION)*(table_count+1))))
>> + DBUG_RETURN(TRUE);
>> +
>> + if (!(join->best_positions=
>> + (POSITION *)join->thd->alloc(sizeof(POSITION)*(table_count+1))))
>> + DBUG_RETURN(TRUE);
>> +
>
> this, or like is done in the previous lines for stat/stat_ref/table_vector:
> just do a bunch of
> x= join->thd->alloc()
> and one final test
> if (!x || !y || !z || !t || etc)
> DBUG_RETURN(TRUE);
I prefer to backout early when an error is detected, so I would rather stick to
my original fix.
A more elegant way is:
if (!(a= allocate_a()) ||
!(b= allocate_b()) ||
!(c= allocate_c()))
DBUG_RETURN(TRUE);
but the reason that I do not like to use it is that it becomes difficult to step
into the different allocate_X functions in a debugger. Besides, you do not know
which of the functions that caused the error.
>
> Ok to push.
>
> As this patch is touching memory allocation and using MEM_ROOT (via
> thd->alloc()), I suggest running all optimizer tests with --ps-protocol
> --valgrind to see whether there's a problem before pushing.
Yes, good advice. Validated that no more valgrind warnings occur after my fix :(
> Thanks.
