maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

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); }

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