maria-developers team mailing list archive

[Igor Babaev <igor@xxxxxxxxxxxx>]

On 04/24/2011 01:51 AM, Sergei Golubchik wrote:
> Hi, Igor!
> 
> On Apr 24, Igor Babaev wrote:
>>
>> When debugging I discovered the following bad code in sql_select.cc of
>> the current 5.3 tree:
> 
>>             tmp= table->file->read_time(key, 1,
>>                                         min(tmp,s->worst_seeks)-1); <=  !!!!! ?
>>             tmp*= record_count;
>>
>> gannotate says that it appeared in the code after your merge with 5.2.
>>
>> The fact is that 5.2 contains the correct expression here
>>    min(tmp,s->worst_seeks)
>> not
>>    min(tmp,s->worst_seeks)-1
>>
>> As a result if the number of records is 1 then min(tmp,s->worst_seeks)-1
>> is equal to 0 and we pass 0 as the last parameter to
>> table->file->read_time. Naturally it returns 0.
>>
>> Will you fix this problem or you'd rather want me to do it?
> 
> In 5.2 it is
> 
>   tmp= record_count*min(tmp,s->worst_seeks);
> 
> In 5.3, record_count* is moved to a separate line, so it should just be
> 
>   tmp= min(tmp,s->worst_seeks);
> 
> But it uses table->file->read_time(). Which is defined as
> 
>   virtual double read_time(uint index, uint ranges, ha_rows rows)
>     { return rows2double(ranges+rows); }

In the implementation of innodb/xtradb (ha_innobase::read_time)
if the parameter rows = 0 , the function returns 0 :(

	if (rows <= 2) {

		return((double) rows);
	}


Regards,
Igor.

> 
> so, when ranges=1 and we pass rows=min(tmp,s->worst_seeks)-1, we get the
> same value of min(tmp,s->worst_seeks) as the result.
> 
> This was the change of introducing handler::keyread_time(), and at the
> same time using handler::read_time() where appropriate, to have all
> values comparable.
> 
> Regards,
> Sergei