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 <[email protected]
> <mailto:[email protected]>> wrote:
>
> Hello Alexey,
>
> Please review a patch for MDEV-12199.
>
> Thanks!
>
>
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
