Thread: Re: [HACKERS] regressin failure on latest CVS
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 [mailto:pgman@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 [mailto:pgman@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
>
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 [mailto:pgman@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 [mailto:pgman@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
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 [mailto:pgman@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 [mailto:pgman@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