The following bug has been logged online:

Bug reference: 3431
Logged by: Pelle Johansson
Email address: pelle@morth.org
PostgreSQL version: 8.2.3
Operating system: Linux 2.6
Description: age() gets the days wrong
Details:

This might be a known issue but i couldn't find it reported before...

The age() function seem to work by first counting months until less than a
month remains to to the second argument, then counting days left. This
doesn't give the correct result, as shown by this example:

# select column1, age(column1, '2006-11-02'), date '2006-11-02' +
age(column1, '2006-11-02') from (values ('2007-01-31'::date),
('2007-02-01')) as alias;
column1 | age | ?column?
------------+----------------+---------------------
2007-01-31 | 2 mons 29 days | 2007-01-31 00:00:00
2007-02-01 | 2 mons 29 days | 2007-01-31 00:00:00
(2 rows)

For the second row, the age should be '2 mons 30 days', which is what you
need to add to '2006-11-02' to get back to '2007-02-01'.

The - operator does not have this problem, so it can be used as a
workaround, but not in all situations.

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

"Pelle Johansson" <pelle@morth.org> writes:
> The age() function seem to work by first counting months until less than a
> month remains to to the second argument, then counting days left. This
> doesn't give the correct result, as shown by this example:

age() isn't intended to give the same result as minus, so I'm not
convinced that this behavior is wrong. I'm more worried about what
applications we might break by changing it --- age() has always worked
the way it does now.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

"Pelle Johansson" <pelle@morth.org> writes:
> The age() function seem to work by first counting months until less than a
> month remains to to the second argument, then counting days left. This
> doesn't give the correct result, as shown by this example:

> # select column1, age(column1, '2006-11-02'), date '2006-11-02' +
> age(column1, '2006-11-02') from (values ('2007-01-31'::date),
> ('2007-02-01')) as alias;
> column1 | age | ?column?
> ------------+----------------+---------------------
> 2007-01-31 | 2 mons 29 days | 2007-01-31 00:00:00
> 2007-02-01 | 2 mons 29 days | 2007-01-31 00:00:00
> (2 rows)

I took another look at this example. I believe what is actually going
wrong here is that when timestamp_age converts a month into an
equivalent number of days, it uses the number of days in the first
month of the interval it's dealing with (ie, the month containing
the earlier of the two dates). This is just wrong, because interval
addition adds months first and then days. The appropriate conversion
to use is actually the length of the next-to-last month of the interval.

As an example, 8.2 and CVS HEAD produce

regression=# select age('2007-03-14', '2007-02-15');
age
---------
27 days
(1 row)

which is reasonable, but

regression=# select age('2007-04-14', '2007-02-15');
age
---------------
1 mon 27 days
(1 row)

is not so reasonable, nor is

regression=# select age('2007-03-14', '2007-01-15');
age
---------------
1 mon 30 days
(1 row)

If we change the code to use the next-to-last month of the interval
then these two cases produce '1 mon 30 days' and '1 mon 27 days'
respectively.

Another problem is that the code isn't doing the propagate-to-next-field
bit for negative fractional seconds. Hence it produces

regression=# select age('2007-02-14 01:00:00', '2007-01-15 01:00:00.4');
age
----------------------
30 days -00:00:00.40
(1 row)

which is maybe not incorrect, but certainly fairly inconsistent with

regression=# select age('2007-02-14 01:00:00', '2007-01-15 01:00:01');
age
------------------
29 days 23:59:59
(1 row)


Hence I propose the attached patch. This does not change any existing
regression test outputs, but it does change the example given in the
documentation: age(timestamp '2001-04-10', timestamp '1957-06-13')
will now produce '43 years 9 mons 28 days' not 27 days. Which actually
is correct if you try to add back the result to timestamp '1957-06-13'.
It also appears to fix Palle's example:

regression=# select column1, age(column1, '2006-11-02'), date '2006-11-02' +
age(column1, '2006-11-02') from (values ('2007-01-31'::date),
('2007-02-01')) as alias;
column1 | age | ?column?
------------+----------------+---------------------
2007-01-31 | 2 mons 29 days | 2007-01-31 00:00:00
2007-02-01 | 2 mons 30 days | 2007-02-01 00:00:00
(2 rows)

As I said earlier, I'm worried about changing the behavior of a function
that's been around for so long, so I'm disinclined to back-patch this.
But it seems like a reasonable change to make in 8.3. Comments?

regards, tom lane


Index: timestamp.c
================================================== =================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.179
diff -c -r1.179 timestamp.c
*** timestamp.c 6 Jul 2007 04:15:59 -0000 1.179
--- timestamp.c 8 Jul 2007 19:45:04 -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;
***************
*** 3082,3097 ****
tm->tm_mday--;
}

! while (tm->tm_mday < 0)
{
if (dt1 < dt2)
{
! tm->tm_mday += day_tab[isleap(tm1->tm_year)][tm1->tm_mon - 1];
! tm->tm_mon--;
}
else
{
! tm->tm_mday += day_tab[isleap(tm2->tm_year)][tm2->tm_mon - 1];
tm->tm_mon--;
}
}
--- 3094,3130 ----
tm->tm_mday--;
}

! /*
! * day-to-month conversion is tricky because variable. For each
! * decrement in tm_mon, we should adjust tm_mday by the length of
! * the next-to-last month(s) of the original time interval.
! * This corresponds to the notion that interval addition will add
! * months first, then days.
! */
! if (tm->tm_mday < 0)
{
+ int end_year;
+ int end_mon;
+
if (dt1 < dt2)
{
! end_year = tm2->tm_year;
! end_mon = tm2->tm_mon;
}
else
{
! end_year = tm1->tm_year;
! end_mon = tm1->tm_mon;
! }
!
! while (tm->tm_mday < 0)
! {
! if (--end_mon <= 0)
! {
! end_mon = MONTHS_PER_YEAR;
! end_year--;
! }
! tm->tm_mday += day_tab[isleap(end_year)][end_mon - 1];
tm->tm_mon--;
}
}
***************
*** 3158,3163 ****
--- 3191,3197 ----
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 ****
--- 3212,3228 ----
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;
***************
*** 3196,3211 ****
tm->tm_mday--;
}

! while (tm->tm_mday < 0)
{
if (dt1 < dt2)
{
! tm->tm_mday += day_tab[isleap(tm1->tm_year)][tm1->tm_mon - 1];
! tm->tm_mon--;
}
else
{
! tm->tm_mday += day_tab[isleap(tm2->tm_year)][tm2->tm_mon - 1];
tm->tm_mon--;
}
}
--- 3241,3277 ----
tm->tm_mday--;
}

! /*
! * day-to-month conversion is tricky because variable. For each
! * decrement in tm_mon, we should adjust tm_mday by the length of
! * the next-to-last month(s) of the original time interval.
! * This corresponds to the notion that interval addition will add
! * months first, then days.
! */
! if (tm->tm_mday < 0)
{
+ int end_year;
+ int end_mon;
+
if (dt1 < dt2)
{
! end_year = tm2->tm_year;
! end_mon = tm2->tm_mon;
}
else
{
! end_year = tm1->tm_year;
! end_mon = tm1->tm_mon;
! }
!
! while (tm->tm_mday < 0)
! {
! if (--end_mon <= 0)
! {
! end_mon = MONTHS_PER_YEAR;
! end_year--;
! }
! tm->tm_mday += day_tab[isleap(end_year)][end_mon - 1];
tm->tm_mon--;
}
}


---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Sorry, I see there was later discussion.

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

Tom Lane wrote:
> "Pelle Johansson" <pelle@morth.org> writes:
> > The age() function seem to work by first counting months until less than a
> > month remains to to the second argument, then counting days left. This
> > doesn't give the correct result, as shown by this example:
>
> > # select column1, age(column1, '2006-11-02'), date '2006-11-02' +
> > age(column1, '2006-11-02') from (values ('2007-01-31'::date),
> > ('2007-02-01')) as alias;
> > column1 | age | ?column?
> > ------------+----------------+---------------------
> > 2007-01-31 | 2 mons 29 days | 2007-01-31 00:00:00
> > 2007-02-01 | 2 mons 29 days | 2007-01-31 00:00:00
> > (2 rows)
>
> I took another look at this example. I believe what is actually going
> wrong here is that when timestamp_age converts a month into an
> equivalent number of days, it uses the number of days in the first
> month of the interval it's dealing with (ie, the month containing
> the earlier of the two dates). This is just wrong, because interval
> addition adds months first and then days. The appropriate conversion
> to use is actually the length of the next-to-last month of the interval.
>
> As an example, 8.2 and CVS HEAD produce
>
> regression=# select age('2007-03-14', '2007-02-15');
> age
> ---------
> 27 days
> (1 row)
>
> which is reasonable, but
>
> regression=# select age('2007-04-14', '2007-02-15');
> age
> ---------------
> 1 mon 27 days
> (1 row)
>
> is not so reasonable, nor is
>
> regression=# select age('2007-03-14', '2007-01-15');
> age
> ---------------
> 1 mon 30 days
> (1 row)
>
> If we change the code to use the next-to-last month of the interval
> then these two cases produce '1 mon 30 days' and '1 mon 27 days'
> respectively.
>
> Another problem is that the code isn't doing the propagate-to-next-field
> bit for negative fractional seconds. Hence it produces
>
> regression=# select age('2007-02-14 01:00:00', '2007-01-15 01:00:00.4');
> age
> ----------------------
> 30 days -00:00:00.40
> (1 row)
>
> which is maybe not incorrect, but certainly fairly inconsistent with
>
> regression=# select age('2007-02-14 01:00:00', '2007-01-15 01:00:01');
> age
> ------------------
> 29 days 23:59:59
> (1 row)
>
>
> Hence I propose the attached patch. This does not change any existing
> regression test outputs, but it does change the example given in the
> documentation: age(timestamp '2001-04-10', timestamp '1957-06-13')
> will now produce '43 years 9 mons 28 days' not 27 days. Which actually
> is correct if you try to add back the result to timestamp '1957-06-13'.
> It also appears to fix Palle's example:
>
> regression=# select column1, age(column1, '2006-11-02'), date '2006-11-02' +
> age(column1, '2006-11-02') from (values ('2007-01-31'::date),
> ('2007-02-01')) as alias;
> column1 | age | ?column?
> ------------+----------------+---------------------
> 2007-01-31 | 2 mons 29 days | 2007-01-31 00:00:00
> 2007-02-01 | 2 mons 30 days | 2007-02-01 00:00:00
> (2 rows)
>
> As I said earlier, I'm worried about changing the behavior of a function
> that's been around for so long, so I'm disinclined to back-patch this.
> But it seems like a reasonable change to make in 8.3. Comments?
>
> regards, tom lane
>

Content-Description: age.patch

> Index: timestamp.c
> ================================================== =================
> RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
> retrieving revision 1.179
> diff -c -r1.179 timestamp.c
> *** timestamp.c 6 Jul 2007 04:15:59 -0000 1.179
> --- timestamp.c 8 Jul 2007 19:45:04 -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;
> ***************
> *** 3082,3097 ****
> tm->tm_mday--;
> }
>
> ! while (tm->tm_mday < 0)
> {
> if (dt1 < dt2)
> {
> ! tm->tm_mday += day_tab[isleap(tm1->tm_year)][tm1->tm_mon - 1];
> ! tm->tm_mon--;
> }
> else
> {
> ! tm->tm_mday += day_tab[isleap(tm2->tm_year)][tm2->tm_mon - 1];
> tm->tm_mon--;
> }
> }
> --- 3094,3130 ----
> tm->tm_mday--;
> }
>
> ! /*
> ! * day-to-month conversion is tricky because variable. For each
> ! * decrement in tm_mon, we should adjust tm_mday by the length of
> ! * the next-to-last month(s) of the original time interval.
> ! * This corresponds to the notion that interval addition will add
> ! * months first, then days.
> ! */
> ! if (tm->tm_mday < 0)
> {
> + int end_year;
> + int end_mon;
> +
> if (dt1 < dt2)
> {
> ! end_year = tm2->tm_year;
> ! end_mon = tm2->tm_mon;
> }
> else
> {
> ! end_year = tm1->tm_year;
> ! end_mon = tm1->tm_mon;
> ! }
> !
> ! while (tm->tm_mday < 0)
> ! {
> ! if (--end_mon <= 0)
> ! {
> ! end_mon = MONTHS_PER_YEAR;
> ! end_year--;
> ! }
> ! tm->tm_mday += day_tab[isleap(end_year)][end_mon - 1];
> tm->tm_mon--;
> }
> }
> ***************
> *** 3158,3163 ****
> --- 3191,3197 ----
> 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 ****
> --- 3212,3228 ----
> 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;
> ***************
> *** 3196,3211 ****
> tm->tm_mday--;
> }
>
> ! while (tm->tm_mday < 0)
> {
> if (dt1 < dt2)
> {
> ! tm->tm_mday += day_tab[isleap(tm1->tm_year)][tm1->tm_mon - 1];
> ! tm->tm_mon--;
> }
> else
> {
> ! tm->tm_mday += day_tab[isleap(tm2->tm_year)][tm2->tm_mon - 1];
> tm->tm_mon--;
> }
> }
> --- 3241,3277 ----
> tm->tm_mday--;
> }
>
> ! /*
> ! * day-to-month conversion is tricky because variable. For each
> ! * decrement in tm_mon, we should adjust tm_mday by the length of
> ! * the next-to-last month(s) of the original time interval.
> ! * This corresponds to the notion that interval addition will add
> ! * months first, then days.
> ! */
> ! if (tm->tm_mday < 0)
> {
> + int end_year;
> + int end_mon;
> +
> if (dt1 < dt2)
> {
> ! end_year = tm2->tm_year;
> ! end_mon = tm2->tm_mon;
> }
> else
> {
> ! end_year = tm1->tm_year;
> ! end_mon = tm1->tm_mon;
> ! }
> !
> ! while (tm->tm_mday < 0)
> ! {
> ! if (--end_mon <= 0)
> ! {
> ! end_mon = MONTHS_PER_YEAR;
> ! end_year--;
> ! }
> ! tm->tm_mday += day_tab[isleap(end_year)][end_mon - 1];
> tm->tm_mon--;
> }
> }

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org

--
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. +

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

I don't see this as applied yet.

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

Tom Lane wrote:
> "Pelle Johansson" <pelle@morth.org> writes:
> > The age() function seem to work by first counting months until less than a
> > month remains to to the second argument, then counting days left. This
> > doesn't give the correct result, as shown by this example:
>
> > # select column1, age(column1, '2006-11-02'), date '2006-11-02' +
> > age(column1, '2006-11-02') from (values ('2007-01-31'::date),
> > ('2007-02-01')) as alias;
> > column1 | age | ?column?
> > ------------+----------------+---------------------
> > 2007-01-31 | 2 mons 29 days | 2007-01-31 00:00:00
> > 2007-02-01 | 2 mons 29 days | 2007-01-31 00:00:00
> > (2 rows)
>
> I took another look at this example. I believe what is actually going
> wrong here is that when timestamp_age converts a month into an
> equivalent number of days, it uses the number of days in the first
> month of the interval it's dealing with (ie, the month containing
> the earlier of the two dates). This is just wrong, because interval
> addition adds months first and then days. The appropriate conversion
> to use is actually the length of the next-to-last month of the interval.
>
> As an example, 8.2 and CVS HEAD produce
>
> regression=# select age('2007-03-14', '2007-02-15');
> age
> ---------
> 27 days
> (1 row)
>
> which is reasonable, but
>
> regression=# select age('2007-04-14', '2007-02-15');
> age
> ---------------
> 1 mon 27 days
> (1 row)
>
> is not so reasonable, nor is
>
> regression=# select age('2007-03-14', '2007-01-15');
> age
> ---------------
> 1 mon 30 days
> (1 row)
>
> If we change the code to use the next-to-last month of the interval
> then these two cases produce '1 mon 30 days' and '1 mon 27 days'
> respectively.
>
> Another problem is that the code isn't doing the propagate-to-next-field
> bit for negative fractional seconds. Hence it produces
>
> regression=# select age('2007-02-14 01:00:00', '2007-01-15 01:00:00.4');
> age
> ----------------------
> 30 days -00:00:00.40
> (1 row)
>
> which is maybe not incorrect, but certainly fairly inconsistent with
>
> regression=# select age('2007-02-14 01:00:00', '2007-01-15 01:00:01');
> age
> ------------------
> 29 days 23:59:59
> (1 row)
>
>
> Hence I propose the attached patch. This does not change any existing
> regression test outputs, but it does change the example given in the
> documentation: age(timestamp '2001-04-10', timestamp '1957-06-13')
> will now produce '43 years 9 mons 28 days' not 27 days. Which actually
> is correct if you try to add back the result to timestamp '1957-06-13'.
> It also appears to fix Palle's example:
>
> regression=# select column1, age(column1, '2006-11-02'), date '2006-11-02' +
> age(column1, '2006-11-02') from (values ('2007-01-31'::date),
> ('2007-02-01')) as alias;
> column1 | age | ?column?
> ------------+----------------+---------------------
> 2007-01-31 | 2 mons 29 days | 2007-01-31 00:00:00
> 2007-02-01 | 2 mons 30 days | 2007-02-01 00:00:00
> (2 rows)
>
> As I said earlier, I'm worried about changing the behavior of a function
> that's been around for so long, so I'm disinclined to back-patch this.
> But it seems like a reasonable change to make in 8.3. Comments?
>
> regards, tom lane
>


--
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. +

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings