Re: BUG #2056: to_char no long takes time as input? - Mailing list pgsql-bugs

From Bruce Momjian
Subject Re: BUG #2056: to_char no long takes time as input?
Date
Msg-id 200511232104.jANL4Tq12184@candle.pha.pa.us
Whole thread Raw
In response to Re: BUG #2056: to_char no long takes time as input?  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: BUG #2056: to_char no long takes time as input?
List pgsql-bugs
Tom Lane wrote:
> "Nick Addington" <adding@math.wisc.edu> writes:
> > The following code works in 8.0.4 but fails in 8.1.0:
> > select to_char('1:00 pm'::time,'HH:MM AM');
>
> I'm inclined to think that disallowing AM/PM for intervals was a mistake
> --- if we allow both HH and HH24 (which we do) then the various flavors
> of AM/PM seem reasonable to allow as well.
>
> Bruce, did you have a specific rationale for that?
>
> I notice the code now specifically forces HH to be treated as HH24 in
> the interval case, which was not so before 8.1; we would need to revert
> that change as well to continue supporting TIME cases reasonably.

When I coded it I was thinking it doesn't make sense to allow "AM"/"PM" for
something like "5 hours".  I disallowed anything that tied to a specific
timestamp value, even BC/AD.

I didn't realize "time" was used as input to to_char() as an interval.
I can see "time" as anchoring to midnight and we could then allow AM/PM.
I see your issue with HH/HH24, but I wanted this to work:

    test=> select to_char('14 hours'::interval, 'HH');
     to_char
    ---------
     14
    (1 row)

With the HH/HH24 change that is going to return 2.  Do interval folks
know they would have to use HH24 for intervals?  It doesn't seem 100%
clear, but I suppose we could just add a documentation about it, because
consider this:

    test=> select to_char('44 hours'::interval, 'HH');
     to_char
    ---------
     44
    (1 row)

With "HH" changed that would return 32.  Ewe.  Here is the formatting.c
code:

                if (is_interval)
                    sprintf(inout, "%0*d", S_FM(suf) ? 0 : 2, tm->tm_hour);
                else
                    sprintf(inout, "%0*d", S_FM(suf) ? 0 : 2,
                            tm->tm_hour == 0 ? 12 :
                          tm->tm_hour < 13 ? tm->tm_hour : tm->tm_hour - 12);

Should we subtract 12 only if the time is < 24.  That also seems
strange.  Also, a zero hour interval to HH would return 12, not 0.

It also seems strange to use HH24 for a value that might be greater than
24 (hours), while 'time' can not.

Anyway, I am thinking the easiest solution is to allow 'time' work
easily with HH, and to document that HH24 needs to be used for
intervals.

If this is what we want, I can make the change.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: Re: Incorrect column identifer using AS in SELECT statement on a VIEW.
Next
From: Tom Lane
Date:
Subject: Re: BUG #2056: to_char no long takes time as input?