Re: bzr commit into mysql-5.1-bugteam branch (davi:2994) Bug#45261

[Davi Arnaut]

On 7/6/09 2:21 PM, Sergei Golubchik wrote:
> Hi, Davi!
>
> On Jul 04, Davi Arnaut wrote:
>> # At a local mysql-5.1-bugteam repository of davi
>>
>>   2994 Davi Arnaut	2009-07-04
>>        Bug#45261: Crash, stored procedure + decimal
>>
>>        The problem was that creating a DECIMAL column from decimal
>>        value could lead to a assertion as decimal values can have
>>        a higher precision than those attached to a table. The assert
>>        could be triggered by creating a table from a decimal with
>>        a large (>  30) scale.
>>
>>        The solution is to ensure that truncation procedure is executed
>>        when deducing a DECIMAL column from a decimal value of higher
>>        precision. If the integer part is equal to or bigger than the
>>        maximum precision for the DECIMAL type (65), the integer part
>
> Didn't we agree that DECIMAL_MAX_PRECISION should be 73, not 65 ?
> (81-8, not 81-8*2)

It can be used store up to 73, but DECIMAL_MAX_PRECISION sets the 
maximum allowed precision for a new DECIMAL, not how much it could store.

It appears to be intended to cap the precision at 65 maximum for the new 
decimal type. Otherwise, it would be possible to create DECIMAL(M,0) 
where M > 65. Do you want to allow this?

>
>>        is truncated to fit and the fractional becomes zero. Otherwise,
>>        the fractional part is truncated to fit into the space left
>>        after the integer part is copied.
>>
>>        This patch borrows code and ideas from Martin Hansson's patch.
>
>> === modified file 'sql/sql_select.cc'
>> *** a/sql/sql_select.cc	2009-06-26 19:57:42 +0000
>> --- b/sql/sql_select.cc	2009-07-04 04:05:37 +0000
>> ***************
>> *** 9368,9407 ****
>>        break;
>>      case DECIMAL_RESULT:
>>      {
>> -     uint8 dec= item->decimals;
>> -     uint8 intg= ((Item_decimal *) item)->decimal_precision() - dec;
>> -     uint32 len= item->max_length;
>> -
>>        /*
>> !       Trying to put too many digits overall in a DECIMAL(prec,dec)
>> !       will always throw a warning. We must limit dec to
>> !       DECIMAL_MAX_SCALE however to prevent an assert() later.
>> !     */
>>
>> !     if (dec>  0)
>> !     {
>> !       signed int overflow;
>>
>> !       dec= min(dec, DECIMAL_MAX_SCALE);
>>
>> !       /*
>> !         If the value still overflows the field with the corrected dec,
>> !         we'll throw out decimals rather than integers. This is still
>> !         bad and of course throws a truncation warning.
>> !         +1: for decimal point
>> !       */
>>
>> !       overflow= my_decimal_precision_to_length(intg + dec, dec,
>> !                                                item->unsigned_flag) - len;
>>
>> !       if (overflow>  0)
>> !         dec= max(0, dec - overflow);            // too long, discard fract
>> !       else
>> !         len -= item->decimals - dec;            // corrected value fits
>>        }
>>
>> !     new_field= new Field_new_decimal(len, maybe_null, item->name,
>> !                                      dec, item->unsigned_flag);
>>        break;
>>      }
>>      case ROW_RESULT:
>> --- 9368,9446 ----
>>        break;
>>      case DECIMAL_RESULT:
>>      {
>>        /*
>> !       The MySQL DECIMAL data type has a characteristic that needs to be
>> !       taken into account when deducing the type from a Item_decimal.
>>
>> !       But first, let's briefly recap what is the new MySQL DECIMAL type:
>>
>> !       The declaration syntax for a decimal is DECIMAL(M,D), where:
>>
>> !       * M is the maximum number of digits (the precision).
>> !         It has a range of 1 to 65.
>> !       * D is the number of digits to the right of the decimal separator (the
> scale).
>> !         It has a range of 0 to 30 and must be no larger than M.
>>
>> !       D and M are used to determine the storage requirements for the integer
>> !       and fractional parts of each value. The integer part is to the left of
>> !       the decimal separator and to the right is the fractional part. Hence:
>>
>> !       M is the number of digits for the integer and fractional part.
>> !       D is the number of digits for the fractional part.
>> !
>> !       Consequently, M - D is the number of digits for the integer part. For
>> !       example, a DECIMAL(20,10) column has ten digits on either side of the
>> !       decimal separator.
>> !
>> !       The characteristic that needs to be taken into account is that the
>> !       backing type for Item_decimal is a my_decimal that has a higher
> precision
>> !       (DECIMAL_MAX_POSSIBLE_PRECISION, see my_decimal.h) than DECIMAL.
>> !
>> !       Drawing a comparison between my_decimal and DECIMAL:
>> !
>> !       * M has a range of 1 to 81.
>> !       * D has a range of 0 to 72.
>> !
>> !       The difference in range is that the fractional part must always be on
>> !       on a group boundary, leaving one group for the integer part. Since each
>> !       group is 9 (DIG_PER_DEC1) digits and there are 9 (DECIMAL_BUFF_LENGTH)
>> !       groups, the fractional part is limited to 72 digits.
>
> No, having no groups for tne integer part is ok, the decimal point still
> be at the group boundary. So, D can be up to 81:

Yes, consequence o shifting the paragraphs around. I intended to say 
that if there is a integer part like 1, the decimal part is limited to 
72 digits.

> mysql>  select
> .123456789012345678901234567890123456789012345678901234567890123456789012345678901;
>
> +-------------------------------------------------------------------------------------+
> | .123456789012345678901234567890123456789012345678901234567890123456789012345678901 
> |
>
> +-------------------------------------------------------------------------------------+
> | 0.123456789012345678901234567890123456789012345678901234567890123456789012345678901
> |
>
> +-------------------------------------------------------------------------------------+
> 1 row in set (0.00 sec)
>
>> !       Although the backing type for a DECIMAL is also my_decimal, every time
>> !       a my_decimal is stored in a DECIMAL field, the precision and scale are
>> !       explicitly capped at 65 (DECIMAL_MAX_PRECISION) and 30
> (DECIMAL_MAX_SCALE)
>> !       digits, following my_decimal truncation procedure (FIX_INTG_FRAC_ERROR).
>> !     */
>> !
>> !     uint8 scale= item->decimals;
>> !     uint8 precision= ((Item_decimal *) item)->decimal_precision();
>
> why uint8 ?

That's what the code was using. AFAICS, no reason in particular.

>> !     /*
>> !       Employ a procedure along the lines of the my_decimal truncation process:
>> !       - If the integer part is equal to or bigger than the maximum precision:
>> !         Truncate integer part to fit and the fractional becomes zero.
>> !       - Otherwise:
>> !         Truncate fractional part to fit.
>> !     */
>> !
>> !     uint intg= precision - scale;
>> !
>> !     if (intg>= DECIMAL_MAX_PRECISION)
>> !     {
>> !       intg= DECIMAL_MAX_PRECISION;
>> !       scale= 0;
>>        }
>> +     else
>> +     {
>> +       uint room= min(DECIMAL_MAX_PRECISION - intg, DECIMAL_MAX_SCALE);
>> +       if (scale>  room)
>> +         scale= room;
>> +     }
>
> where's a truncation warning/note generated ?
>

Once a value is stored in the field.

Regards,

-- Davi Arnaut