maria-developers team mailing list archive
Mailing list archive
MDEV-4309, second patch: review
This is a review for MDEV-4309, second patch:
The name "memcmp_func" is confusing, because the function
has nothing to do with memcmp. Did you mean memcpy actually?
The function should be renamed to something like "memcpy_field_value".
memcmp_func() still makes one virtual call.
Consider a scenario:
- Item_field::setup_cpy_optimizer() is called.
- It calls memcmp_possible(), which returns FALSE, which means field_conv()
will be called for every row.
- field_conv() will still call memcpy_field_value(), even if we know that it
will return false.
Why not add a field_conv_incompatible() and make field_conv look like this:
The name "cpy_optimizer" is very cryptic. Is "Fast_field_copier" better?
class Item_field has this member:
I don't see how it is useful without its corresponding 'cpy_optimizer' data.
cpy_optimizer is only present inside ORDER structure which has both:
If ORDER has both members, why have the Item_field::cpy_optimizer_setup ?
Let's discuss this on IRC.
Sergei Petrunia, Software Developer
MariaDB | Skype: sergefp | Blog: http://s.petrunia.net/blog