List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:October 27 2010 11:02am
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (Olav.Sandstaa:3264)
Bug#52660
View as plain text  
Hi Tor,

Thanks for your review comments.

On 27/10/2010 09:37, Tor Didriksen wrote:
> Hi Olav
>
> Please document (in the .h file) the interface of
> virtual Item *idx_cond_push(uint keyno, Item* idx_cond)
> Especially the return value.

I agree and will include documentation for this in my next patch. (Note: 
This new interface method was introduced as part of WL#2474/2475 which 
includes multiple other new methods to the handler interface for 
MRR/ICP. Most of the other new member functions are not documented 
either. Seems like we should do a separate task to improve the interface 
documentation for ICP/MRR).

>
> suggestion:
>   for (uint k= 0; !key_contains_blob && k < key->key_parts; ++k)
>   {
>     const KEY_PART_INFO *key_part= &key->key_part[k];
>     if (key_part->key_part_flag & HA_BLOB_PART)
>       key_contains_blob= true;
>   }
>

I normally do not like to put too much logic in the for-loop's condition 
but I agree that it is better than to having the break statement here.
Another alternative for avoiding the break statement could be to have a 
return statement like this instead:

for (uint k= 0; k<  key->key_parts; ++k)
   {
     const KEY_PART_INFO *key_part=&key->key_part[k];
     if (key_part->key_part_flag&  HA_BLOB_PART)
       return idx_cond_arg;
   }


This way I would also avoid the entire key_contains_blob variable. What 
do you think? I will include the one you prefer in my next patch.

> suggestion:
> let qq= SELECT c1 FROM t3 WHERE c1 >= 'c-1004=w' and c1 <= 'c-1006=w' 
> and i1 > 2;
> eval EXPLAIN $qq;
> eval $qq;

Thanks for this suggestion. Will update this in the next version of the 
patch.

Olav

Thread
bzr commit into mysql-next-mr-bugfixing branch (Olav.Sandstaa:3264) Bug#52660Olav.Sandstaa19 Oct
  • Re: bzr commit into mysql-next-mr-bugfixing branch(Olav.Sandstaa:3264) Bug#52660Tor Didriksen27 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch (Olav.Sandstaa:3264)Bug#52660Olav Sandstaa27 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch(Olav.Sandstaa:3264) Bug#52660Olav SandstÃ¥1 Nov
      • Re: bzr commit into mysql-next-mr-bugfixing branch(Olav.Sandstaa:3264) Bug#52660Tor Didriksen2 Nov
  • Re: bzr commit into mysql-next-mr-bugfixing branch (Olav.Sandstaa:3264)Bug#52660Jorgen Loland28 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch(Olav.Sandstaa:3264) Bug#52660Olav SandstÃ¥1 Nov
      • Re: bzr commit into mysql-next-mr-bugfixing branch (Olav.Sandstaa:3264)Bug#52660Jorgen Loland2 Nov