That still does not fix it for me.

This patch is still using a computed float value (month_remainder and
day_remainder), which is cauing the rounding errors.

There are now 6 machines on the build farm that are failing from the
rounding:
Wallaroo (OSX/G4), asp(AIX/powerpc), viper(FC3/x86_64),
platypus(FBSD/amd64), kookaburra(AIX/powerpc), oriole(FC4/x86).

All of these are using enable-integer-datetimes.

What was wrong with the patch I sent? I am doing as much as I can with
integer math.

-rocco

> -----Original Message-----
> From: Bruce Momjian [mailtogman@candle.pha.pa.us]
> Sent: Saturday, July 23, 2005 2:51 PM
> To: Rocco Altier
> Cc: Michael Glaesemann; pgsql-patches@postgresql.org; ohp@pyrenet.fr
> Subject: Re: [HACKERS] regressin failure on latest CVS
>
>
>
> Would you please try the attached patch and let me know if it
> fixes the
> problem? I avoided accumulating into a float8.
>
> --------------------------------------------------------------
> -------------
>
> Rocco Altier wrote:
> > This still does not fix the problem.
> >
> > I had done my patch to try to mimic the way 8.0 had handled the math
> > with the remainders, but to carry it over another bucket (day).
> >
> > The problem that I see is that we are taking day_remainder and
> > multiplying by USECS_PER_DAY. Which is a double * int64,
> thus there is
> > the precision loss there.
> >
> > I think initial division by the factor can't be helped, but
> repeatedly
> > doing more floating point math on with it is causing the
> rounding error.
> >
> > Thanks,
> > -rocco
> >
> > > -----Original Message-----
> > > From: Bruce Momjian [mailtogman@candle.pha.pa.us]
> > > Sent: Saturday, July 23, 2005 10:54 AM
> > > To: Rocco Altier
> > > Cc: Michael Glaesemann; pgsql-patches@postgresql.org;
> > > pgsql-hackers@postgresql.org; ohp@pyrenet.fr
> > > Subject: Re: [HACKERS] regressin failure on latest CVS
> > >
> > >
> > > Rocco Altier wrote:
> > > > This patch fixes the interval regression on my AIX box
> > > (kookaburra) by
> > > > only doing integer math on the interval, instead of
> > > float/double math.
> > > >
> > > > I think this is the correct way to handle this, since it's
> > > an integer
> > > > data type.
> > > >
> > > > I don't know if it will fix Olivier's problem, since I
> > > wasn't able to
> > > > reproduce it.
> > > >
> > >
> > > I have changed the way I compute the remainder values ---
> instead of
> > > using multiplication, I use division and then subtraction.
> > > This should
> > > fix your rounding problem. Looking at your fix, I don't see
> > > how adding
> > > USECS changes things because the factor is already a float,
> > > but I think
> > > the problem was more the way I was computing the remainders.
> > >
> > > Patch attached --- let me know if it does not fix your problem.
> > >
> > > --------------------------------------------------------------
> >
> >
> > ---------------------------(end of
> broadcast)---------------------------
> > TIP 2: Don't 'kill -9' the postmaster
> >
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman@candle.pha.pa.us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square,
> Pennsylvania 19073
>

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Rocco Altier wrote:
> That still does not fix it for me.
>
> This patch is still using a computed float value (month_remainder and
> day_remainder), which is cauing the rounding errors.
>
> There are now 6 machines on the build farm that are failing from the
> rounding:
> Wallaroo (OSX/G4), asp(AIX/powerpc), viper(FC3/x86_64),
> platypus(FBSD/amd64), kookaburra(AIX/powerpc), oriole(FC4/x86).
>
> All of these are using enable-integer-datetimes.
>
> What was wrong with the patch I sent? I am doing as much as I can with
> integer math.

OK, new patch, please test. Yea, I know your patch works, but we
usually keep at it until we have a clean, understood solution. Thanks.

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

>
> -rocco
>
> > -----Original Message-----
> > From: Bruce Momjian [mailtogman@candle.pha.pa.us]
> > Sent: Saturday, July 23, 2005 2:51 PM
> > To: Rocco Altier
> > Cc: Michael Glaesemann; pgsql-patches@postgresql.org; ohp@pyrenet.fr
> > Subject: Re: [HACKERS] regressin failure on latest CVS
> >
> >
> >
> > Would you please try the attached patch and let me know if it
> > fixes the
> > problem? I avoided accumulating into a float8.
> >
> > --------------------------------------------------------------
> > -------------
> >
> > Rocco Altier wrote:
> > > This still does not fix the problem.
> > >
> > > I had done my patch to try to mimic the way 8.0 had handled the math
> > > with the remainders, but to carry it over another bucket (day).
> > >
> > > The problem that I see is that we are taking day_remainder and
> > > multiplying by USECS_PER_DAY. Which is a double * int64,
> > thus there is
> > > the precision loss there.
> > >
> > > I think initial division by the factor can't be helped, but
> > repeatedly
> > > doing more floating point math on with it is causing the
> > rounding error.
> > >
> > > Thanks,
> > > -rocco
> > >
> > > > -----Original Message-----
> > > > From: Bruce Momjian [mailtogman@candle.pha.pa.us]
> > > > Sent: Saturday, July 23, 2005 10:54 AM
> > > > To: Rocco Altier
> > > > Cc: Michael Glaesemann; pgsql-patches@postgresql.org;
> > > > pgsql-hackers@postgresql.org; ohp@pyrenet.fr
> > > > Subject: Re: [HACKERS] regressin failure on latest CVS
> > > >
> > > >
> > > > Rocco Altier wrote:
> > > > > This patch fixes the interval regression on my AIX box
> > > > (kookaburra) by
> > > > > only doing integer math on the interval, instead of
> > > > float/double math.
> > > > >
> > > > > I think this is the correct way to handle this, since it's
> > > > an integer
> > > > > data type.
> > > > >
> > > > > I don't know if it will fix Olivier's problem, since I
> > > > wasn't able to
> > > > > reproduce it.
> > > > >
> > > >
> > > > I have changed the way I compute the remainder values ---
> > instead of
> > > > using multiplication, I use division and then subtraction.
> > > > This should
> > > > fix your rounding problem. Looking at your fix, I don't see
> > > > how adding
> > > > USECS changes things because the factor is already a float,
> > > > but I think
> > > > the problem was more the way I was computing the remainders.
> > > >
> > > > Patch attached --- let me know if it does not fix your problem.
> > > >
> > > > --------------------------------------------------------------
> > >
> > >
> > > ---------------------------(end of
> > broadcast)---------------------------
> > > TIP 2: Don't 'kill -9' the postmaster
> > >
> >
> > --
> > Bruce Momjian | http://candle.pha.pa.us
> > pgman@candle.pha.pa.us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup. | Newtown Square,
> > Pennsylvania 19073
> >
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Index: src/backend/utils/adt/timestamp.c
================================================== =================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.145
diff -c -c -r1.145 timestamp.c
*** src/backend/utils/adt/timestamp.c 23 Jul 2005 14:53:21 -0000 1.145
--- src/backend/utils/adt/timestamp.c 23 Jul 2005 19:34:39 -0000
***************
*** 2294,2300 ****
{
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
! double month_remainder, day_remainder;
Interval *result;

result = (Interval *) palloc(sizeof(Interval));
--- 2294,2300 ----
{
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
! double month_remainder;
Interval *result;

result = (Interval *) palloc(sizeof(Interval));
***************
*** 2308,2327 ****
result->day = span->day / factor;
result->time = span->time / factor;

- /* Compute remainders */
- month_remainder = span->month / factor - result->month;
- day_remainder = span->day / factor - result->day;
-
/* Cascade fractions to lower units */
! /* fractional months full days into days */
result->day += month_remainder * DAYS_PER_MONTH;
- /* fractional months partial days into time */
- day_remainder += (month_remainder * DAYS_PER_MONTH) - (int)(month_remainder * DAYS_PER_MONTH);

#ifdef HAVE_INT64_TIMESTAMP
! result->time += day_remainder * USECS_PER_DAY;
#else
! result->time += day_remainder * SECS_PER_DAY;
result->time = JROUND(result->time);
#endif

--- 2308,2327 ----
result->day = span->day / factor;
result->time = span->time / factor;

/* Cascade fractions to lower units */
! month_remainder = (span->month / factor - result->month);
result->day += month_remainder * DAYS_PER_MONTH;

+ /* fractional months and partial days into time */
#ifdef HAVE_INT64_TIMESTAMP
! /* divide by 'factor' at the end to avoid rounding errors */
! result->time += ((span->day - (int)(span->day * factor) +
! ((span->month - result->month * factor) * DAYS_PER_MONTH))
! * USECS_PER_DAY) / factor;
#else
! result->time += (span->day / factor - (int)(span->day / factor) +
! month_remainder * DAYS_PER_MONTH -
! (int)(month_remainder * DAYS_PER_MONTH)) * SECS_PER_DAY;
result->time = JROUND(result->time);
#endif



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

http://archives.postgresql.org

Bruce Momjian wrote:
> Rocco Altier wrote:
> > That still does not fix it for me.
> >
> > This patch is still using a computed float value (month_remainder and
> > day_remainder), which is cauing the rounding errors.
> >
> > There are now 6 machines on the build farm that are failing from the
> > rounding:
> > Wallaroo (OSX/G4), asp(AIX/powerpc), viper(FC3/x86_64),
> > platypus(FBSD/amd64), kookaburra(AIX/powerpc), oriole(FC4/x86).
> >
> > All of these are using enable-integer-datetimes.
> >
> > What was wrong with the patch I sent? I am doing as much as I can with
> > integer math.
>
> OK, new patch, please test. Yea, I know your patch works, but we
> usually keep at it until we have a clean, understood solution. Thanks.

Sorry, new patch, previous was broken. These are not in CVS yet.

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


> > > -----Original Message-----
> > > From: Bruce Momjian [mailtogman@candle.pha.pa.us]
> > > Sent: Saturday, July 23, 2005 2:51 PM
> > > To: Rocco Altier
> > > Cc: Michael Glaesemann; pgsql-patches@postgresql.org; ohp@pyrenet.fr
> > > Subject: Re: [HACKERS] regressin failure on latest CVS
> > >
> > >
> > >
> > > Would you please try the attached patch and let me know if it
> > > fixes the
> > > problem? I avoided accumulating into a float8.
> > >
> > > --------------------------------------------------------------
> > > -------------
> > >
> > > Rocco Altier wrote:
> > > > This still does not fix the problem.
> > > >
> > > > I had done my patch to try to mimic the way 8.0 had handled the math
> > > > with the remainders, but to carry it over another bucket (day).
> > > >
> > > > The problem that I see is that we are taking day_remainder and
> > > > multiplying by USECS_PER_DAY. Which is a double * int64,
> > > thus there is
> > > > the precision loss there.
> > > >
> > > > I think initial division by the factor can't be helped, but
> > > repeatedly
> > > > doing more floating point math on with it is causing the
> > > rounding error.
> > > >
> > > > Thanks,
> > > > -rocco
> > > >
> > > > > -----Original Message-----
> > > > > From: Bruce Momjian [mailtogman@candle.pha.pa.us]
> > > > > Sent: Saturday, July 23, 2005 10:54 AM
> > > > > To: Rocco Altier
> > > > > Cc: Michael Glaesemann; pgsql-patches@postgresql.org;
> > > > > pgsql-hackers@postgresql.org; ohp@pyrenet.fr
> > > > > Subject: Re: [HACKERS] regressin failure on latest CVS
> > > > >
> > > > >
> > > > > Rocco Altier wrote:
> > > > > > This patch fixes the interval regression on my AIX box
> > > > > (kookaburra) by
> > > > > > only doing integer math on the interval, instead of
> > > > > float/double math.
> > > > > >
> > > > > > I think this is the correct way to handle this, since it's
> > > > > an integer
> > > > > > data type.
> > > > > >
> > > > > > I don't know if it will fix Olivier's problem, since I
> > > > > wasn't able to
> > > > > > reproduce it.
> > > > > >
> > > > >
> > > > > I have changed the way I compute the remainder values ---
> > > instead of
> > > > > using multiplication, I use division and then subtraction.
> > > > > This should
> > > > > fix your rounding problem. Looking at your fix, I don't see
> > > > > how adding
> > > > > USECS changes things because the factor is already a float,
> > > > > but I think
> > > > > the problem was more the way I was computing the remainders.
> > > > >
> > > > > Patch attached --- let me know if it does not fix your problem.
> > > > >
> > > > > --------------------------------------------------------------
> > > >
> > > >
> > > > ---------------------------(end of
> > > broadcast)---------------------------
> > > > TIP 2: Don't 'kill -9' the postmaster
> > > >
> > >
> > > --
> > > Bruce Momjian | http://candle.pha.pa.us
> > > pgman@candle.pha.pa.us | (610) 359-1001
> > > + If your life is a hard drive, | 13 Roberts Road
> > > + Christ can be your backup. | Newtown Square,
> > > Pennsylvania 19073
> > >
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 1: if posting/reading through Usenet, please send an appropriate
> > subscribe-nomail command to majordomo@postgresql.org so that your
> > message can get through to the mailing list cleanly
> >
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman@candle.pha.pa.us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania 19073

> Index: src/backend/utils/adt/timestamp.c
> ================================================== =================
> RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
> retrieving revision 1.145
> diff -c -c -r1.145 timestamp.c
> *** src/backend/utils/adt/timestamp.c 23 Jul 2005 14:53:21 -0000 1.145
> --- src/backend/utils/adt/timestamp.c 23 Jul 2005 19:34:39 -0000
> ***************
> *** 2294,2300 ****
> {
> Interval *span = PG_GETARG_INTERVAL_P(0);
> float8 factor = PG_GETARG_FLOAT8(1);
> ! double month_remainder, day_remainder;
> Interval *result;
>
> result = (Interval *) palloc(sizeof(Interval));
> --- 2294,2300 ----
> {
> Interval *span = PG_GETARG_INTERVAL_P(0);
> float8 factor = PG_GETARG_FLOAT8(1);
> ! double month_remainder;
> Interval *result;
>
> result = (Interval *) palloc(sizeof(Interval));
> ***************
> *** 2308,2327 ****
> result->day = span->day / factor;
> result->time = span->time / factor;
>
> - /* Compute remainders */
> - month_remainder = span->month / factor - result->month;
> - day_remainder = span->day / factor - result->day;
> -
> /* Cascade fractions to lower units */
> ! /* fractional months full days into days */
> result->day += month_remainder * DAYS_PER_MONTH;
> - /* fractional months partial days into time */
> - day_remainder += (month_remainder * DAYS_PER_MONTH) - (int)(month_remainder * DAYS_PER_MONTH);
>
> #ifdef HAVE_INT64_TIMESTAMP
> ! result->time += day_remainder * USECS_PER_DAY;
> #else
> ! result->time += day_remainder * SECS_PER_DAY;
> result->time = JROUND(result->time);
> #endif
>
> --- 2308,2327 ----
> result->day = span->day / factor;
> result->time = span->time / factor;
>
> /* Cascade fractions to lower units */
> ! month_remainder = (span->month / factor - result->month);
> result->day += month_remainder * DAYS_PER_MONTH;
>
> + /* fractional months and partial days into time */
> #ifdef HAVE_INT64_TIMESTAMP
> ! /* divide by 'factor' at the end to avoid rounding errors */
> ! result->time += ((span->day - (int)(span->day * factor) +
> ! ((span->month - result->month * factor) * DAYS_PER_MONTH))
> ! * USECS_PER_DAY) / factor;
> #else
> ! result->time += (span->day / factor - (int)(span->day / factor) +
> ! month_remainder * DAYS_PER_MONTH -
> ! (int)(month_remainder * DAYS_PER_MONTH)) * SECS_PER_DAY;
> result->time = JROUND(result->time);
> #endif
>

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

--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Index: src/backend/utils/adt/timestamp.c
================================================== =================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.145
diff -c -c -r1.145 timestamp.c
*** src/backend/utils/adt/timestamp.c 23 Jul 2005 14:53:21 -0000 1.145
--- src/backend/utils/adt/timestamp.c 23 Jul 2005 19:51:21 -0000
***************
*** 2294,2300 ****
{
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
! double month_remainder, day_remainder;
Interval *result;

result = (Interval *) palloc(sizeof(Interval));
--- 2294,2300 ----
{
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
! double month_remainder;
Interval *result;

result = (Interval *) palloc(sizeof(Interval));
***************
*** 2308,2327 ****
result->day = span->day / factor;
result->time = span->time / factor;

- /* Compute remainders */
- month_remainder = span->month / factor - result->month;
- day_remainder = span->day / factor - result->day;
-
/* Cascade fractions to lower units */
! /* fractional months full days into days */
result->day += month_remainder * DAYS_PER_MONTH;
- /* fractional months partial days into time */
- day_remainder += (month_remainder * DAYS_PER_MONTH) - (int)(month_remainder * DAYS_PER_MONTH);

#ifdef HAVE_INT64_TIMESTAMP
! result->time += day_remainder * USECS_PER_DAY;
#else
! result->time += day_remainder * SECS_PER_DAY;
result->time = JROUND(result->time);
#endif

--- 2308,2327 ----
result->day = span->day / factor;
result->time = span->time / factor;

/* Cascade fractions to lower units */
! month_remainder = (span->month / factor - result->month);
result->day += month_remainder * DAYS_PER_MONTH;

+ /* fractional months and partial days into time */
#ifdef HAVE_INT64_TIMESTAMP
! /* divide by 'factor' at the end to avoid rounding errors */
! result->time += ((span->day - (int)(result->day * factor) +
! ((span->month - (int)(result->month * factor)) * DAYS_PER_MONTH))
! * USECS_PER_DAY) / factor;
#else
! result->time += (span->day / factor - (int)(span->day / factor) +
! month_remainder * DAYS_PER_MONTH -
! (int)(month_remainder * DAYS_PER_MONTH)) * SECS_PER_DAY;
result->time = JROUND(result->time);
#endif



---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster