Hi, Davi!
ok to push.
one comment needs to be clarified, see below.
and I think you should change 65 to 73, again, see below.
Thanks!
On Jul 06, Davi Arnaut wrote:
> On 7/6/09 2:21 PM, Sergei Golubchik wrote:
>> 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?
It's defined as
/**
maximum guaranteed precision of number in decimal digits (number of our
digits * number of decimal digits in one our big digit - number of decimal
digits in one our big digit decreased by 1 (because we always put decimal
point on the border of our big digits))
*/
#define DECIMAL_MAX_PRECISION (DECIMAL_MAX_POSSIBLE_PRECISION - 8*2)
so I *think* that the intention was "how much it could store", as 65 is
simply a miscalculation.
Holyfoot (who implemented that) seems to agree, I've just asked him on
irc.
>>> 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.
>>>
>>> === 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 ****
>>> !
>>> ! 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.
okay, I presume you'll fix the comment.
>>> ! 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.
could you change to int or uint please ?
Regards / Mit vielen GrЭъen,
Sergei
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect
/_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB MЭnchen 161028
<___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten
GeschДftsfЭhrer: Thomas Schroeder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin HДring
