Re: [BUGS] BUG #3431: age() gets the days wrong - Mailing list pgsql-patches

I have applied the attached patch that documents the age() behavior,
plus fixes the mismatch sign for seconds by using part of Tom's earlier
patch.

I agree we want to keep the symmetry we have.  We can call this item
closed.

---------------------------------------------------------------------------

Tom Lane wrote:
> Pelle Johansson <pelle@morth.org> writes:
> > If you were to use the result for subtracting from the first value,
> > instead of adding to the second, the conditions are reversed. It's
> > not really as obvious as I first thought whether there's 2 months and
> > 29 days or 2 months and 30 days between 2006-11-02 and 2007-02-01...
>
> Hmm, that's a really good point; perhaps the original author was
> thinking of it in those terms, in which case using the first month of
> the interval is indeed sane.  (Almost: I believe that the loop can
> iterate more than once, and then you need to look to the second month
> etc.  The code's not doing that, so there's still a corner-case bug,
> plus the fsec issue.)
>
> Other than that corner case, it seems the behavior we currently have is
>     if x > y, age() produces a positive interval such that
>         x - age(x, y) = y
>     if x < y, age() produces a negative interval such that
>         y + age(x, y) = x
>
> Are we satisfied with just documenting that, or do we want to change it,
> and if so to what?
>
> As the code currently stands, we have the symmetry property
>     age(x,y) = - age(y,x)
> for all x,y.  I don't think we can preserve that if we try to simplify
> the relationship to interval addition/subtraction.
>
> Comments?
>
>             regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: You can help support the PostgreSQL project by donating at
>
>                 http://www.postgresql.org/about/donate

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + 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.382
diff -c -c -r1.382 func.sgml
*** doc/src/sgml/func.sgml    6 Jun 2007 23:00:35 -0000    1.382
--- doc/src/sgml/func.sgml    18 Jul 2007 03:09:42 -0000
***************
*** 5895,5900 ****
--- 5895,5911 ----
     <literal>CST7CDT</literal>.
    </para>

+   <para>
+    Note that when the <function>age</> function operates on multi-month
+    intervals, <productname>PostgreSQL</> adds days to the earlier date
+    until full months can be added.  This yields a different result than
+    adding full months first if the interval crosses from one month to the
+    next.  For example, <literal>age('2004-06-01', '2004-04-30')</> yeilds
+    <literal>1 mon 1 day</> using the <productname>PostgreSQL</> method,
+    while adding the month first would yield <literal>1 mon 2 days</>
+    because May has 31 days, while April has only 30.
+   </para>
+
    <sect2 id="functions-datetime-extract">
     <title><function>EXTRACT</function>, <function>date_part</function></title>

Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.179
diff -c -c -r1.179 timestamp.c
*** src/backend/utils/adt/timestamp.c    6 Jul 2007 04:15:59 -0000    1.179
--- src/backend/utils/adt/timestamp.c    18 Jul 2007 03:09:44 -0000
***************
*** 3044,3050 ****
      if (timestamp2tm(dt1, NULL, tm1, &fsec1, NULL, NULL) == 0 &&
          timestamp2tm(dt2, NULL, tm2, &fsec2, NULL, NULL) == 0)
      {
!         fsec = (fsec1 - fsec2);
          tm->tm_sec = tm1->tm_sec - tm2->tm_sec;
          tm->tm_min = tm1->tm_min - tm2->tm_min;
          tm->tm_hour = tm1->tm_hour - tm2->tm_hour;
--- 3044,3051 ----
      if (timestamp2tm(dt1, NULL, tm1, &fsec1, NULL, NULL) == 0 &&
          timestamp2tm(dt2, NULL, tm2, &fsec2, NULL, NULL) == 0)
      {
!         /* form the symbolic difference */
!         fsec = fsec1 - fsec2;
          tm->tm_sec = tm1->tm_sec - tm2->tm_sec;
          tm->tm_min = tm1->tm_min - tm2->tm_min;
          tm->tm_hour = tm1->tm_hour - tm2->tm_hour;
***************
*** 3064,3069 ****
--- 3065,3081 ----
              tm->tm_year = -tm->tm_year;
          }

+         /* propagate any negative fields into the next higher field */
+         while (fsec < 0)
+         {
+ #ifdef HAVE_INT64_TIMESTAMP
+             fsec += USECS_PER_SEC;
+ #else
+             fsec += 1.0;
+ #endif
+             tm->tm_sec--;
+         }
+
          while (tm->tm_sec < 0)
          {
              tm->tm_sec += SECS_PER_MINUTE;
***************
*** 3158,3163 ****
--- 3170,3176 ----
      if (timestamp2tm(dt1, &tz1, tm1, &fsec1, &tzn, NULL) == 0 &&
          timestamp2tm(dt2, &tz2, tm2, &fsec2, &tzn, NULL) == 0)
      {
+         /* form the symbolic difference */
          fsec = fsec1 - fsec2;
          tm->tm_sec = tm1->tm_sec - tm2->tm_sec;
          tm->tm_min = tm1->tm_min - tm2->tm_min;
***************
*** 3178,3183 ****
--- 3191,3207 ----
              tm->tm_year = -tm->tm_year;
          }

+         /* propagate any negative fields into the next higher field */
+         while (fsec < 0)
+         {
+ #ifdef HAVE_INT64_TIMESTAMP
+             fsec += USECS_PER_SEC;
+ #else
+             fsec += 1.0;
+ #endif
+             tm->tm_sec--;
+         }
+
          while (tm->tm_sec < 0)
          {
              tm->tm_sec += SECS_PER_MINUTE;