maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

Hi, Sanja!

The idea of the fix is correct. See a couple of comments below.

On Mar 14, sanja@xxxxxxxxxxxx wrote:
> message:
>   MDEV-4272 fix.
>   
>   Incorrect NULL value handling in Item_func_conv_charset fixed.

> === modified file 'sql/item_strfunc.cc'
> --- a/sql/item_strfunc.cc	2013-01-28 12:36:05 +0000
> +++ b/sql/item_strfunc.cc	2013-03-14 13:22:32 +0000
> @@ -2987,7 +2987,7 @@ String *Item_func_conv_charset::val_str(
>      return null_value ? 0 : &str_value;
>    String *arg= args[0]->val_str(str);
>    uint dummy_errors;
> -  if (!arg)
> +  if (!arg || args[0]->null_value)

it's redundant. One of these conditions is enough, no need to check
both.

>    {
>      null_value=1;
>      return 0;
> 
> === modified file 'sql/item_strfunc.h'
> --- a/sql/item_strfunc.h	2013-01-15 18:07:46 +0000
> +++ b/sql/item_strfunc.h	2013-03-14 13:22:32 +0000
> @@ -859,25 +859,45 @@ public:
>    {
>      if (args[0]->result_type() == STRING_RESULT)
>        return Item_str_func::val_int();
> -    return args[0]->val_int();
> +    {
> +      longlong res= args[0]->val_int();
> +      if ((null_value= args[0]->null_value))
> +        return 0;
> +      return res;
> +    }

Why did you add an extra pair of curly brackets? it's also redundant.

Regards,
Sergei