On Tue, Sep 9, 2008 at 6:46 AM, Brendan Jurd <direvus@gmail.com> wrote:
> On Tue, Sep 9, 2008 at 9:04 PM, Brendan Jurd <direvus@gmail.com> wrote:
>> On Tue, Sep 9, 2008 at 7:29 PM, Martijn van Oosterhout
>> <kleptog@svana.org> wrote:
>>> The use of palloc/pfree in this routine seems excessive. Does len have
>>> upper bound? If so a simple array will do it.
>>>
>>
Good catch Martijn!
>> I suppose I could define a constant FORMATNODE_MAX_LEN, make it
>> something like 10 and just use that for copying the string, rather
>> than palloc(). I'll give it a try.
>>
>
> Turns out there was already a relevant constant defined in
> formatting.c: DCH_MAX_ITEM_SIZ, set to 9. So I just used that, +1 for
> the trailing null.
Cool.
>>>
>>> Here you do not note if we didn't convert the entire string. So it
>>> seems you are allowed to provide too few characters, as long as it's
>>> not the last field?
>>
>> That's true. The only way to hit that condition would be to provide a
>> semi-bogus value in a string with no separators between the numbers.
>
> I've now plugged this hole. I added the following error for
> fixed-width inputs that are too short:
>
> postgres=# SELECT to_date('200%1010', 'YYYYMMDD');
> ERROR: invalid value for "YYYY" in source string
> DETAIL: Field requires 4 characters, but only 3 could be parsed.
> HINT: If your source string is not fixed-width, try using the "FM" modifier.
I think thats a big improvement.
> I've attached a new version of the patch (version 3), as well as an
> incremental patch to show the differences between versions 2 and 3.
I looked it over, looks good to me!
> Cheers,
> BJ
>