Re: bk commit into 5.0 tree (evgen:1.2610) BUG#36023

[Sergey Petrunia]

Hi,

On Thu, Apr 24, 2008 at 12:52:02AM +0400, eugene@stripped wrote:
> ChangeSet@stripped, 2008-04-24 00:51:59+04:00, evgen@stripped +3 -0
>   Bug#36023: Incorrect handling of zero length caused an assertion to fail.
>   
>   When a zero length is provided to the my_decimal_length_to_precision
>   function along with unsigned_flag set to false it returns a negative value.
>   For queries that employs temporary tables may cause failed assertion or
>   excessive memory consumption while temporary table creation.
>   
>   Now the my_decimal_length_to_precision and the my_decimal_precision_to_length
>   functions take unsigned_flag into account only if the length/precision
>   argument is non-zero.
> 
>   mysql-test/r/type_decimal.result@stripped, 2008-04-24 00:51:14+04:00,
> evgen@stripped +7 -0
>     Added a test case for the bug#36023: Incorrect handling of zero length caused
>      an assertion to fail.
> 
>   mysql-test/t/type_decimal.test@stripped, 2008-04-24 00:50:29+04:00,
> evgen@stripped +8 -0
>     Added a test case for the bug#36023: Incorrect handling of zero length caused
>      an assertion to fail.
> 
>   sql/my_decimal.h@stripped, 2008-04-24 00:51:32+04:00, evgen@stripped +4 -2
>     Bug#36023: Incorrect handling of zero length caused an assertion to fail.
>     Now the my_decimal_length_to_precision and the my_decimal_precision_to_length
>     functions take unsigned_flag into account only if the length/precision
>     argument is non-zero.
> 
> diff -Nrup a/sql/my_decimal.h b/sql/my_decimal.h
> --- a/sql/my_decimal.h	2007-05-28 02:05:33 +04:00
> +++ b/sql/my_decimal.h	2008-04-24 00:51:32 +04:00
> @@ -164,14 +164,16 @@ inline int check_result_and_overflow(uin
>  inline uint my_decimal_length_to_precision(uint length, uint scale,
>                                             bool unsigned_flag)
>  {
> -  return (uint) (length - (scale>0 ? 1:0) - (unsigned_flag ? 0:1));
> +  return (uint) (length - (scale>0 ? 1:0) -
> +                 (unsigned_flag || !length ? 0:1));
>  }
>  
>  inline uint32 my_decimal_precision_to_length(uint precision, uint8 scale,
>                                               bool unsigned_flag)
>  {
>    set_if_smaller(precision, DECIMAL_MAX_PRECISION);
> -  return (uint32)(precision + (scale>0 ? 1:0) + (unsigned_flag ? 0:1));
> +  return (uint32)(precision + (scale>0 ? 1:0) +
> +                  (unsigned_flag || !precision ? 0:1));
>  }

- Please add a comment saying that the functions need to handle the case
  when precision==0 and what handling is performed in this case.
- AFAIU we assume that precision==0 => scale==0 ? please add 
  DBUG_ASSERT((precision !=0 || scale==0))

Ok to push after the above is addressed.

BR
 Sergey
-- 
Sergey Petrunia, Lead Software Engineer
MySQL AB, www.mysql.com
Office: N/A
Blog: http://s.petrunia.net/blog