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
