Re: to_char("HH12") with intervals - Mailing list pgsql-hackers

Bruce Momjian wrote:
> bruce wrote:
> > bruce wrote:
> > > Dave Page wrote:
> > > > This was posted as a documentation comment:
> > > >
> > > > to_char(interval '0d 0h 12m 44s', 'DD HH MI SS');
> > > > with HH and HH12 will return 12 instead of 0.
> > > >
> > > > Testing on 8.4.1, it does seem to be the case that you get "00 12 12
> > > > 44". Seems bogus to me, but am I and the OP missing something?
> > >
> > > Fixed with the attached patch.  I think HH and HH24 should be the same
> > > for intervals.  It is hard to explain why zero hours should show as
> > > '12' for intervals.
> >
> > Oops, I needed a second patch to fix hours > 12 for intervals.  Patch
> > attached and applied.  It will now report the full hours of the
> > interval.
>
> We currently have this in our documentation:
>
>         <function>to_char(interval)</function> formats <literal>HH</> and
>         <literal>HH12</> as hours in a single day, while <literal>HH24</>
>         can output hours exceeding a single day, e.g., >24.
>
> This seems pretty confusing because HH/HH12 formats as hours in a single
> 1/2 day, 12 hours, and it really does wall-clock time, zero hours is 12,
> and for intervals it does the right thing now and prints the interval
> hours.
>
> We also have these range definitions:
>
>        <row>
>         <entry><literal>HH12</literal></entry>
>         <entry>hour of day (01-12)</entry>
>        </row>
>        <row>
>         <entry><literal>HH24</literal></entry>
>         <entry>hour of day (00-23)</entry>
>        </row>
>
> HH24 could always be > 24 for intervals, and now HH12 can too for
> intervals.
>
> What should be changed here?

I have rethought this and now realize the original code was fine, but
the documentation was very unclear.  I have reverted the patch and added
a C command and documentation updates with examples; attached.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/func.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.505
diff -c -c -r1.505 func.sgml
*** doc/src/sgml/func.sgml    19 Feb 2010 00:15:25 -0000    1.505
--- doc/src/sgml/func.sgml    23 Feb 2010 16:12:37 -0000
***************
*** 5317,5324 ****
       <listitem>
        <para>
          <function>to_char(interval)</function> formats <literal>HH</> and
!         <literal>HH12</> as hours in a single day, while <literal>HH24</>
!         can output hours exceeding a single day, e.g., >24.
        </para>
       </listitem>

--- 5317,5325 ----
       <listitem>
        <para>
          <function>to_char(interval)</function> formats <literal>HH</> and
!         <literal>HH12</> as shown on a 12-hour clock, i.e. zero hours
!         and 36 hours output as <literal>12</>, while <literal>HH24</>
!         outputs the full hour value, which can exceed 23 for intervals.
        </para>
       </listitem>

Index: src/backend/utils/adt/formatting.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v
retrieving revision 1.165
diff -c -c -r1.165 formatting.c
*** src/backend/utils/adt/formatting.c    23 Feb 2010 06:29:01 -0000    1.165
--- src/backend/utils/adt/formatting.c    23 Feb 2010 16:12:37 -0000
***************
*** 2088,2097 ****
                  break;
              case DCH_HH:
              case DCH_HH12:
                  sprintf(s, "%0*d", S_FM(n->suffix) ? 0 : 2,
!                         is_interval ? tm->tm_hour :
!                         tm->tm_hour % (HOURS_PER_DAY / 2) == 0 ?
!                         12 : tm->tm_hour % (HOURS_PER_DAY / 2));
                  if (S_THth(n->suffix))
                      str_numth(s, s, S_TH_TYPE(n->suffix));
                  s += strlen(s);
--- 2088,2097 ----
                  break;
              case DCH_HH:
              case DCH_HH12:
+                 /* display time as shown on a 12-hour clock, even for intervals */
                  sprintf(s, "%0*d", S_FM(n->suffix) ? 0 : 2,
!                         tm->tm_hour % (HOURS_PER_DAY / 2) == 0 ? 12 :
!                         tm->tm_hour % (HOURS_PER_DAY / 2));
                  if (S_THth(n->suffix))
                      str_numth(s, s, S_TH_TYPE(n->suffix));
                  s += strlen(s);