Re: Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Date
Msg-id CAFj8pRBpT5=JqGV6Exu+CnO5OhVACCnqi6Zfs5URy3WSk1Yhfw@mail.gmail.com
Whole thread Raw
In response to Re: Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used  (Dean Rasheed <dean.a.rasheed@gmail.com>)
List pgsql-hackers
2013/3/5 Dean Rasheed <dean.a.rasheed@gmail.com>:
> On 5 March 2013 13:46, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> 2013/3/5 Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>:
>>> Hello,
>>>
>>>> > I think that the only other behavioural glitch I spotted was that it
>>>> > fails to catch one overflow case, which should generate an "out of
>>>> > ranger" error:
>>>> >
>>>> > select format('|%*s|', -2147483648, 'foo');
>>>> > Result: |foo|
>>>> >
>>>> > because -(-2147483648) = 0 in signed 32-bit integers.
>>>
>>> Ouch. Thanks for pointing out.
>>>
>>>> fixed - next other overflow check added
>>>
>>> Your change shown below seems assuming that the two's complement
>>> of the most negative number in integer types is identical to
>>> itself, and looks working as expected at least on
>>> linux/x86_64. But C standard defines it as undefined, (As far as
>>> I hear :-).
>>>
>>> |               if (width != 0)
>>> |               {
>>> |                       int32 _width = -width;
>>> |
>>> |                       if (SAMESIGN(width, _width))
>>> |                               ereport(ERROR,
>>>
>>
>> this pattern was used elsewhere in pg
>>
>>> Instead, I think we can deny it by simply comparing with
>>> INT_MIN. I modified the patch like so and put some modifications
>>> on styling.
>>
>> ook - I have not enough expirience with this topic and I cannot say
>> what is preferred.
>>
>>>
>>> Finally, enlargeStringInfo fails just after for large numbers. We
>>> might should keep it under certain length to get rid of memory
>>> exhaustion.
>>
>> I though about it, but I don't know a correct value - probably any
>> width specification higher 1MB will be bogus and can be blocked ?? Our
>> VARLENA max size is 1GB so with should not be higher than 1GB ever.
>>
>> what do you thinking about these limits?
>>
>
> I think it's fine as it is.
>
> It's no different from repeat() for example. We allow repeat('a',
> 1000000000) so allowing format('%1000000000s', '') seems reasonable,
> although probably not very useful.
>
> Upping either beyond 1GB generates an out of memory error, which also
> seems reasonable -- I can't imagine why you would want such a long
> string.
>
> Regards,
> Dean

ok

Pavel



pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: Suggested new CF status: "Pending Discussion"
Next
From: Alvaro Herrera
Date:
Subject: Re: Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used