Re: BUG #2056: to_char no long takes time as input?

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Nick Addington <adding(at)math(dot)wisc(dot)edu>, pgsql-bugs(at)postgresql(dot)org
Subject: Re: BUG #2056: to_char no long takes time as input?
Date: 2005-11-23 21:04:29
Message-ID: 200511232104.jANL4Tq12184@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-patches

Tom Lane wrote:
> "Nick Addington" <adding(at)math(dot)wisc(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2005-11-23 21:22:34 Re: BUG #2056: to_char no long takes time as input?
Previous Message Dave Cramer 2005-11-23 20:52:43 Re: [JDBC] BUG #2060: Issue with Data base connection

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2005-11-23 21:22:34 Re: BUG #2056: to_char no long takes time as input?
Previous Message Alvaro Herrera 2005-11-23 15:34:30 Re: Space management for PGresult