maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

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