Re: Bug 1500

Tom Lane wrote:

>Lyubomir Petrov <lpetrov(at)sysmaster(dot)com> writes:
>
>
>>I have found what is causing the crash described in Bug 1500. Now I
>>would like to fix it, but need opinions about what is the correct behaviour.
>>
>>
>
>Yeah, I just came to the same conclusion a little while ago:
>http://archives.postgresql.org/pgsql-hackers/2005-03/msg00908.php
>
>
>
>>Also the general to_char() Interval formatting seems broken anyway.
>>
>>
>
>Karel Zak has stated repeatedly that interval_to_char is fundamentally
>wrong and should be removed. I'm not sure it's quite as bad as that,
>but it does seem that a different set of formatting codes is needed for
>intervals as opposed to timestamps. Textual 'MON' doesn't even make any
>sense for intervals really, AFAICS. I could see displaying an interval
>in terms of '4 months', but 'April' makes no sense.
>
>Does Oracle have to_char for intervals, and if so how do they define it?
>
>Anyway, even if we think it's broken enough to remove going forward,
>we need some kind of stopgap fix to prevent the coredump in existing
>releases.
>
> regards, tom lane
>
>---------------------------(end of broadcast)---------------------------
>TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>
>.
>
>
>
Tom,

Well, I can see how the to_char functionality can be very useful for
intervals - one can get the interval in days only, months and days, etc.
But I think that the format specifications that convert to strings
should be disallowed for intervals (Mon, Month, etc...).

If we decide just to ignore the non-supported format code we can
1) make dch_date aware that it is called for interval and limit the
choices (ignore the attempt to show textual name representation for
example)
2) just ignore the attempt to show month name on invalid value in struct
pg_tm.

In the second case we'll need to change only this file several times
using something like (this is good to be there anyway because of the
array indexing):

case DCH_Mon:
+ if (tm->tm_mon > 0) {
+ strcpy(inout, months[tm->tm_mon - 1]);
+ return 2;
+ }
+ return -1;
+
- strcpy(inout, months[tm->tm_mon - 1]);
- return 2;

The first case will probably have more impact. I think we can go with 2)
for 8.0.2 and 1) for 8.1.

Oracle has to_char() on intervals, but generally does not allow fancy
formatting (limited format specifications only - FF, TZD, TZH, TZM, and
TZR - which are not very useful anyway).

Regards,
Lyubomir Petrov