Re: [HACKERS] Interval aggregate regression failure - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: [HACKERS] Interval aggregate regression failure
Date
Msg-id 200608301708.k7UH8jL01607@momjian.us
Whole thread Raw
In response to Re: [HACKERS] Interval aggregate regression failure (expected seems  (Michael Glaesemann <grzm@seespotcode.net>)
Responses Re: [HACKERS] Interval aggregate regression failure
List pgsql-patches
Michael Glaesemann wrote:
> > Yea, just an optimization, but I was worried that the computations
> > might
> > throw problems for certain numbers, so I figured I would only
> > trigger it
> > when necessary.
>
> Thanks for the explanation. Helps me know I might actually be
> learning this.
>
> > Patch attached.  It also fixes a regression test output too.
>
> Thanks for the patch. I'll look at it more closely tonight.

OK, here is a much nicer patch.  The fix is to do no rounding, but to
find the number of days before applying the factor adjustment.  It
passes all your tests here:

    test=> select '41 mon 10:00:00'::interval / 10 as "pos";
              pos
    ------------------------
     4 mons 3 days 01:00:00
    (1 row)

    test=> select interval '41 mon 12 days 360:00' * 0.3 as product_a
         , interval '41 mon -12 days -360:00' * 0.3 as product_b
         , interval '-41 mon 12 days 360:00' * 0.3 as product_c
         , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
            product_a         |        product_b         |
    product_c          |          product_d
    --------------------------+--------------------------+-----------------------------+------------------------------

     1 year 12 days 122:24:00 | 1 year 6 days -122:24:00 | -1 years -6 days
    +122:24:00 | -1 years -12 days -122:24:00
    (1 row)

    test=> select interval '41 mon 12 days 360:00' / 10 as quotient_a
         , interval '41 mon -12 days -360:00' / 10 as quotient_b
         , interval '-41 mon 12 days 360:00' / 10 as quotient_c
         , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
           quotient_a       |       quotient_b        |        quotient_c
         |        quotient_d
    ------------------------+-------------------------+---------------------------+---------------------------

     4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2 days
    +40:48:00 | -4 mons -4 days -40:48:00
    (1 row)

What do you see on your platform?

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c    13 Jul 2006 16:49:16 -0000    1.165
--- src/backend/utils/adt/timestamp.c    30 Aug 2006 17:08:12 -0000
***************
*** 2496,2501 ****
--- 2496,2502 ----
                  day_remainder,
                  month_remainder_days;
      Interval   *result;
+     int32        orig_month = span->month;

      result = (Interval *) palloc(sizeof(Interval));

***************
*** 2516,2523 ****
       * using justify_hours and/or justify_days.
       */

!     /* fractional months full days into days */
!     month_remainder_days = month_remainder * DAYS_PER_MONTH;
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
--- 2517,2533 ----
       * using justify_hours and/or justify_days.
       */

!     /*
!      *    Fractional months full days into days.
!      *
!      *    The remainders suffer from float rounding, so instead of
!      *    doing the computation using just the remainder, we calculate
!      *    the total number of days and subtract.  Specifically, we are
!      *    multipling by DAYS_PER_MONTH before dividing by factor.
!      *    This greatly reduces rounding errors.
!      */
!     month_remainder_days = (orig_month * DAYS_PER_MONTH) * factor -
!             result->month * DAYS_PER_MONTH;
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
***************
*** 2550,2556 ****
                  day_remainder,
                  month_remainder_days;
      Interval   *result;
!
      result = (Interval *) palloc(sizeof(Interval));

      if (factor == 0.0)
--- 2560,2567 ----
                  day_remainder,
                  month_remainder_days;
      Interval   *result;
!     int32        orig_month = span->month;
!
      result = (Interval *) palloc(sizeof(Interval));

      if (factor == 0.0)
***************
*** 2566,2576 ****
      day_remainder -= result->day;

      /*
!      * Handle any fractional parts the same way as in interval_mul.
       */
!
!     /* fractional months full days into days */
!     month_remainder_days = month_remainder * DAYS_PER_MONTH;
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
--- 2577,2587 ----
      day_remainder -= result->day;

      /*
!      *    Fractional months full days into days.  See comment in
!      *    interval_mul().
       */
!     month_remainder_days = (orig_month * DAYS_PER_MONTH) / factor -
!             result->month * DAYS_PER_MONTH;
      result->day += (int32) month_remainder_days;
      /* fractional months partial days into time */
      day_remainder += month_remainder_days - (int32) month_remainder_days;
Index: src/test/regress/expected/interval.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/interval.out,v
retrieving revision 1.15
diff -c -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    30 Aug 2006 17:08:16 -0000
***************
*** 218,224 ****
  select avg(f1) from interval_tbl;
                         avg
  -------------------------------------------------
!  @ 4 years 1 mon 9 days 28 hours 18 mins 23 secs
  (1 row)

  -- test long interval input
--- 218,224 ----
  select avg(f1) from interval_tbl;
                         avg
  -------------------------------------------------
!  @ 4 years 1 mon 10 days 4 hours 18 mins 23 secs
  (1 row)

  -- test long interval input

pgsql-patches by date:

Previous
From: "Jim C. Nasby"
Date:
Subject: Re: [HACKERS] Updatable views
Next
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Interval aggregate regression failure