Re: Interval month, week -> day

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Michael Glaesemann <grzm(at)seespotcode(dot)net>
Cc: pgsql-patches Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Interval month, week -> day
Date: 2006-08-30 04:22:05
Message-ID: 200608300422.k7U4M5x14509@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches


The masks don't need changing.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

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

Michael Glaesemann wrote:
> When trying to improve the rounding in interval_div and interval_mul,
> I came across some behavior that seems counterintuitive to me:
>
> test=# select '1.5 mon'::interval;
> interval
> -----------------
> 1 mon 360:00:00
> (1 row)
>
> With the time/day/month interval struct introduced in 8.1, I'd expect
> this to return '1 mon 15 days'. The reason is that the DecodeInterval
> converts fractional months to time directly, rather than cascading
> first to days.
>
> Similar behavior happens with weeks:
>
> select '1.5 week'::interval;
> interval
> -----------------
> 7 days 84:00:00
> (1 row)
>
> Similarly, I believe should return 10 days 12 hours (7 days + 3.5 days).
>
> I've patched DecodeInterval and the regression tests to check this. I
> think tmask lines need to be updated, but I'm not sure how these work
> so I've left them as is. I'd appreciate it if someone could look at
> these areas in particular.
>
> I think this is a behavior changing bug fix, as it was the intention
> of the Interval struct change to treat days and time differently.
> This patch brings the DecodeInterval function more in line with that
> intention.
>
> Thanks for your consideration.
>
> Michael Glaesemann
> grzm seespotcode net
>
>
> Index: src/backend/utils/adt/datetime.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
> retrieving revision 1.169
> diff -c -r1.169 datetime.c
> *** src/backend/utils/adt/datetime.c 25 Jul 2006 03:51:21 -0000 1.169
> --- src/backend/utils/adt/datetime.c 27 Aug 2006 23:25:53 -0000
> ***************
> *** 2920,2935 ****
> tm->tm_mday += val * 7;
> if (fval != 0)
> {
> ! int sec;
> !
> ! fval *= 7 * SECS_PER_DAY;
> ! sec = fval;
> ! tm->tm_sec += sec;
> #ifdef HAVE_INT64_TIMESTAMP
> ! *fsec += (fval - sec) * 1000000;
> #else
> ! *fsec += fval - sec;
> #endif
> }
> tmask = (fmask & DTK_M(DAY)) ? 0 : DTK_M(DAY);
> break;
> --- 2920,2942 ----
> tm->tm_mday += val * 7;
> if (fval != 0)
> {
> ! int extra_days;
> ! fval *= 7;
> ! extra_days = (int32) fval;
> ! tm->tm_mday += extra_days;
> ! fval -= extra_days;
> ! if (fval != 0)
> ! {
> ! int sec;
> ! fval *= SECS_PER_DAY;
> ! sec = fval;
> ! tm->tm_sec += sec;
> #ifdef HAVE_INT64_TIMESTAMP
> ! *fsec += (fval - sec) * 1000000;
> #else
> ! *fsec += fval - sec;
> #endif
> + }
> }
> tmask = (fmask & DTK_M(DAY)) ? 0 : DTK_M(DAY);
> break;
> ***************
> *** 2938,2953 ****
> tm->tm_mon += val;
> if (fval != 0)
> {
> ! int sec;
> !
> ! fval *= DAYS_PER_MONTH * SECS_PER_DAY;
> ! sec = fval;
> ! tm->tm_sec += sec;
> #ifdef HAVE_INT64_TIMESTAMP
> ! *fsec += (fval - sec) * 1000000;
> #else
> ! *fsec += fval - sec;
> #endif
> }
> tmask = DTK_M(MONTH);
> break;
> --- 2945,2967 ----
> tm->tm_mon += val;
> if (fval != 0)
> {
> ! int day;
> ! fval *= DAYS_PER_MONTH;
> ! day = fval;
> ! tm->tm_mday += day;
> ! fval -= day;
> ! if (fval != 0)
> ! {
> ! int sec;
> ! fval *= SECS_PER_DAY;
> ! sec = fval;
> ! tm->tm_sec += sec;
> #ifdef HAVE_INT64_TIMESTAMP
> ! *fsec += (fval - sec) * 1000000;
> #else
> ! *fsec += fval - sec;
> #endif
> + }
> }
> tmask = DTK_M(MONTH);
> break;
> Index: src/test/regress/expected/interval.out
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/
> interval.out,v
> retrieving revision 1.15
> diff -c -r1.15 interval.out
> *** src/test/regress/expected/interval.out 6 Mar 2006 22:49:17 -0000
> 1.15
> --- src/test/regress/expected/interval.out 27 Aug 2006 23:25:56 -0000
> ***************
> *** 39,44 ****
> --- 39,56 ----
> -1 days +02:03:00
> (1 row)
>
> + SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours";
> + Ten days twelve hours
> + -----------------------
> + 10 days 12:00:00
> + (1 row)
> +
> + SELECT INTERVAL '1.5 months' AS "One month 15 days";
> + One month 15 days
> + -------------------
> + 1 mon 15 days
> + (1 row)
> +
> SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
> 9 years...
> ----------------------------------
> Index: src/test/regress/sql/interval.sql
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/test/regress/sql/interval.sql,v
> retrieving revision 1.9
> diff -c -r1.9 interval.sql
> *** src/test/regress/sql/interval.sql 6 Mar 2006 22:49:17 -0000 1.9
> --- src/test/regress/sql/interval.sql 27 Aug 2006 23:25:56 -0000
> ***************
> *** 11,16 ****
> --- 11,18 ----
> SELECT INTERVAL '-05' AS "Five hours";
> SELECT INTERVAL '-1 +02:03' AS "22 hours ago...";
> SELECT INTERVAL '-1 days +02:03' AS "22 hours ago...";
> + SELECT INTERVAL '1.5 weeks' AS "Ten days twelve hours";
> + SELECT INTERVAL '1.5 months' AS "One month 15 days";
> SELECT INTERVAL '10 years -11 month -12 days +13:14' AS "9 years...";
>
> CREATE TABLE INTERVAL_TBL (f1 interval);
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2006-08-30 04:23:53 Re: Autovacuum on by default?
Previous Message Bruce Momjian 2006-08-30 03:50:40 Re: [HACKERS] Interval aggregate regression failure

Browse pgsql-patches by date

  From Date Subject
Next Message Michael Glaesemann 2006-08-30 05:44:42 Re: [HACKERS] Interval aggregate regression failure (expected seems
Previous Message Bruce Momjian 2006-08-30 03:50:40 Re: [HACKERS] Interval aggregate regression failure