Skip site navigation (1) Skip section navigation (2)

Re: [HACKERS] regressin failure on latest CVS

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Rocco Altier <RoccoA(at)Routescape(dot)com>,Michael Glaesemann <grzm(at)myrealbox(dot)com>,pgsql-patches(at)postgresql(dot)org, ohp(at)pyrenet(dot)fr
Subject: Re: [HACKERS] regressin failure on latest CVS
Date: 2005-07-23 19:53:09
Message-ID: 200507231953.j6NJr9g29946@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-patches
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(at)candle(dot)pha(dot)pa(dot)us] 
> > > Sent: Saturday, July 23, 2005 2:51 PM
> > > To: Rocco Altier
> > > Cc: Michael Glaesemann; pgsql-patches(at)postgresql(dot)org; ohp(at)pyrenet(dot)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(at)candle(dot)pha(dot)pa(dot)us] 
> > > > > Sent: Saturday, July 23, 2005 10:54 AM
> > > > > To: Rocco Altier
> > > > > Cc: Michael Glaesemann; pgsql-patches(at)postgresql(dot)org; 
> > > > > pgsql-hackers(at)postgresql(dot)org; ohp(at)pyrenet(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)postgresql(dot)org so that your
> >        message can get through to the mailing list cleanly
> > 
> 
> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

In response to

pgsql-patches by date

Next:From: Bruce MomjianDate: 2005-07-23 23:29:35
Subject: Re: per user/database connections limit again
Previous:From: Bruce MomjianDate: 2005-07-23 19:35:35
Subject: Re: [HACKERS] regressin failure on latest CVS

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group