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 <bar@mariadb.org <mailto:bar@mariadb.org>> wrote:
Hello Alexey,
Please review a patch for MDEV-12199.
Thanks!