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

2013/3/5 Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>:
> On 5 March 2013 13:46, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> 2013/3/5 Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)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