Hello Alexey, On 03/09/2017 11:38 PM, Alexey Botchkov wrote:

Hi, Alexander.

Some questions about the patch.

I don't get why we have two sets of functions Type_handler_**::+bool Type_handler_int_result::Item_func_abs_fix_length_and_dec and Type_handler_**::+bool Type_handler_int_result::Item_func_neg_fix_length_and_dec

As far as i can see these are equal and we just can have one Type_handler_**::+bool Type_handler_int_result::Item_func_abs_neg_fix_length_and_dec Did I miss something?

They might look similar: bool Type_handler_int_result:: Item_func_abs_fix_length_and_dec(Item_func_abs *item) const { item->fix_length_and_dec_int(); return false; } bool Type_handler_int_result:: Item_func_neg_fix_length_and_dec(Item_func_neg *item) const { item->fix_length_and_dec_int(); return false; } But they have arguments of different pointer types! The former calls Item_func_abs::fix_length_and_dec_int(), and the latter calls Item_func_neg::fix_length_and_dec_int(), and those are different.

Secondly, since Item_func::fix_length_and_dec() was historically 'void' and i don't see you're changing it, why do all these typehandler's 'xxx_fix_length_and_dec' return something? Nobody checks their return anyway.

Currently fix_fields() does this: fix_length_and_dec(); if (thd->is_error()) // An error inside fix_length_and_dec occurred return TRUE; I'm thinking of eventually changing this to: if (fix_length_and_dec()) return true; So I decided to make all Type_handler::xxxx_fix_length_and_dec() return bool right now, so we have to do less changes in the future. Thanks!

Best regards. HF

On Tue, Mar 7, 2017 at 6:15 PM, Alexander Barkov mailto:bar@mariadb.org> wrote:

Hello Alexey,

Please review a patch for MDEV-12199.

Thanks!