Re: [PATCHES] fix integer datetime division rounding error
This fixes the problem for me. Thanks, -rocco > -Original Message- > From: Bruce Momjian [mailto:[EMAIL PROTECTED] > Sent: Sunday, July 24, 2005 12:37 AM > To: Andrew Dunstan > Cc: Patches (PostgreSQL); Rocco Altier > Subject: Re: [PATCHES] fix integer datetime division rounding error > > > Andrew Dunstan wrote: > > > > The attached patch seems to fix the rounding error that is causing > > regression failures on machines with integer datetimes. > (Source of error > > discovered by [EMAIL PROTECTED]).ISTM this code needs to be > given some > > careful analysis - I know it makes my head spin reading it. > > Ah, brilliant! I knew I was missing something fundamental, > and the use > of rint() was it. Strangely enough, the 8.0 code uses rint() in that > function, but for floating point intervals, and the code was buggy, > generating negative time values for division. > > Patch attached and applied. I also improved the interval > multiplication > code. > > -- > 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 2: Don't 'kill -9' the postmaster
[PATCHES] Regression - GNUmakefile - pg_usleep
Attached patch fixes the SHLIB_LINK to add pgport now that pg_usleep is added. This is needed for AIX to resolve symbols at compile time. This is also to be used in conjuction with the other patch I have pending for Makefile.aix to SHLIB_LINK instead of LIBS to compile shared objects. Thanks, -rocco regress_makefile.patch Description: regress_makefile.patch ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] fix integer datetime division rounding error
Andrew Dunstan wrote:
>
> The attached patch seems to fix the rounding error that is causing
> regression failures on machines with integer datetimes. (Source of error
> discovered by [EMAIL PROTECTED]).ISTM this code needs to be given some
> careful analysis - I know it makes my head spin reading it.
Ah, brilliant! I knew I was missing something fundamental, and the use
of rint() was it. Strangely enough, the 8.0 code uses rint() in that
function, but for floating point intervals, and the code was buggy,
generating negative time values for division.
Patch attached and applied. I also improved the interval multiplication
code.
--
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 - 1.145
--- src/backend/utils/adt/timestamp.c 24 Jul 2005 04:34:56 -
***
*** 2244,2281
Datum
interval_mul(PG_FUNCTION_ARGS)
{
! Interval *span1 = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
Interval *result;
- #ifdef HAVE_INT64_TIMESTAMP
- int64 months;
- int64 days;
- #else
- double months;
- double days;
- #endif
-
result = (Interval *) palloc(sizeof(Interval));
! months = span1->month * factor;
! days = span1->day * factor;
#ifdef HAVE_INT64_TIMESTAMP
! result->month = months;
! result->day = days;
! result->time = span1->time * factor;
! result->time += (months - result->month) * INT64CONST(30) *
USECS_PER_DAY;
! result->time += (days - result->day) * INT64CONST(24) * USECS_PER_HOUR;
! #else
! result->month = (int)months;
! result->day = (int)days;
! result->time = JROUND(span1->time * factor);
! /* evaluate fractional months as 30 days */
! result->time += JROUND((months - result->month) * DAYS_PER_MONTH *
SECS_PER_DAY);
! /* evaluate fractional days as 24 hours */
! result->time += JROUND((days - result->day) * HOURS_PER_DAY *
SECS_PER_HOUR);
#endif
PG_RETURN_INTERVAL_P(result);
}
--- 2244,2280
Datum
interval_mul(PG_FUNCTION_ARGS)
{
! 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));
! result->month = span->month * factor;
! result->day = span->day * 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 = rint(span->time * factor +
! day_remainder * USECS_PER_DAY);
! #else
! result->time = JROUND(span->time * factor +
! day_remainder * SECS_PER_DAY);
#endif
+ result = DatumGetIntervalP(DirectFunctionCall1(interval_justify_hours,
+
IntervalPGetDatum(result)));
PG_RETURN_INTERVAL_P(result);
}
***
*** 2284,2292
{
/* Args are float8 and Interval *, but leave them as generic Datum */
Datum factor = PG_GETARG_DATUM(0);
! Datum span1 = PG_GETARG_DATUM(1);
! return DirectFunctionCall2(interval_mul, span1, factor);
}
Datum
--- 2283,2291
{
/* Args are float8 and Interval *, but leave them as generic Datum */
Datum factor = PG_GETARG_DATUM(0);
! Datum span = PG_GETARG_DATUM(1);
! return DirectFunctionCall2(interval_mul, span, factor);
}
Datum
***
*** 2316,2325
/* 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_remainde
Re: [HACKERS] [PATCHES] Patch to fix plpython on OS X
On Sat, Jul 23, 2005 at 07:58:21PM -0400, Andrew Dunstan wrote: > Tom Lane wrote: > >"Jim C. Nasby" <[EMAIL PROTECTED]> writes: > >>I don't think it's a version issue; cuckoo is at 2.4, platypus used to > >>be at 2.3 but I upgraded it to 2.4 to see if that was the issue, but > >>platypus kept working. > > > >Hmm ... if it's *not* a version thing then I really do want to know > >what's causing it. Anyone have an idea why this machine is saying > >'\u80' where everyone else's python says u'\x80' ? > > Another OSX box on buildfarm, wallaroo, is exhibiting the same > behaviour, albeit currently masked by interval regression failures. I suspect this is indeed a Python version issue: http://archives.postgresql.org/pgsql-hackers/2005-07/msg00669.php http://archives.postgresql.org/pgsql-hackers/2005-07/msg00684.php It looks like the Macs have some kind of Python framework that PL/Python is linking against even if a newer version of Python has been installed. Unfortunately I don't have a Mac I could use to do any deeper investigating. The regression tests that are failing are from the patch I submitted about a month ago to fix a core dump in PL/Python: http://archives.postgresql.org/pgsql-patches/2005-06/msg00519.php The tests exercise the error checking that the patch added, doing things that previously caused a segmentation fault but that now raise an exception. Should those tests remain in place? If so, should we rewrite them to avoid the version-specific Python messages (possibly by wrapping them in a PL/pgSQL function that traps the errors), or should we just leave the tests alone now that we think we understand what's happening? -- Michael Fuhr http://www.fuhr.org/~mfuhr/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Patch to fix plpython on OS X
Michael Fuhr <[EMAIL PROTECTED]> writes: >> Tom Lane wrote: >>> Hmm ... if it's *not* a version thing then I really do want to know >>> what's causing it. Anyone have an idea why this machine is saying >>> '\u80' where everyone else's python says u'\x80' ? > The regression tests that are failing are from the patch I submitted > about a month ago to fix a core dump in PL/Python: > http://archives.postgresql.org/pgsql-patches/2005-06/msg00519.php > The tests exercise the error checking that the patch added, doing > things that previously caused a segmentation fault but that now > raise an exception. Should those tests remain in place? If so, > should we rewrite them to avoid the version-specific Python messages > (possibly by wrapping them in a PL/pgSQL function that traps the > errors), or should we just leave the tests alone now that we think > we understand what's happening? Well, if it is just a Python version issue then all we need do is add a variant expected-output file to match. I was just expressing a desire to know that for sure before we wallpaper over the symptom... regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] fix integer datetime division rounding error
The attached patch seems to fix the rounding error that is causing regression failures on machines with integer datetimes. (Source of error discovered by [EMAIL PROTECTED]).ISTM this code needs to be given some careful analysis - I know it makes my head spin reading it. cheers andrew Index: src/backend/utils/adt/timestamp.c === RCS file: /home/cvsmirror/pgsql/src/backend/utils/adt/timestamp.c,v retrieving revision 1.145 diff -c -r1.145 timestamp.c *** src/backend/utils/adt/timestamp.c 23 Jul 2005 14:53:21 - 1.145 --- src/backend/utils/adt/timestamp.c 24 Jul 2005 00:04:08 - *** *** 2319,2325 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); --- 2319,2325 day_remainder += (month_remainder * DAYS_PER_MONTH) - (int)(month_remainder * DAYS_PER_MONTH); #ifdef HAVE_INT64_TIMESTAMP ! result->time += rint(day_remainder * USECS_PER_DAY); #else result->time += day_remainder * SECS_PER_DAY; result->time = JROUND(result->time); ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Patch to fix plpython on OS X
Tom Lane wrote: "Jim C. Nasby" <[EMAIL PROTECTED]> writes: On Tue, Jul 19, 2005 at 10:03:39AM -0400, Tom Lane wrote: "Jim C. Nasby" <[EMAIL PROTECTED]> writes: Attached is a plpython_error_1.out file that will fix cuckoo. What is the reason for the difference in the error message spelling in the first place? Is this a Python version thing (and if so, which version is newer --- that should have pride of place as plpython_error.out I should think)? Or is there some other reason that we need to understand more closely instead of just slapping on a band-aid? I don't think it's a version issue; cuckoo is at 2.4, platypus used to be at 2.3 but I upgraded it to 2.4 to see if that was the issue, but platypus kept working. Hmm ... if it's *not* a version thing then I really do want to know what's causing it. Anyone have an idea why this machine is saying '\u80' where everyone else's python says u'\x80' ? Another OSX box on buildfarm, wallaroo, is exhibiting the same behaviour, albeit currently masked by interval regression failures. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] per user/database connections limit again
> Patch includes only changes to backend, I will make pg_dump, ecpg and
> documentation patches once this is completed and accepted by team.
I am ready to apply this patch. Would you make the additional changes
you suggested? Is there any way to see the limits except to query
pg_authid?
---
Petr Jelinek wrote:
> Stephen Frost wrote:
>
> >This should almost certainly be a pg_database_ownercheck() call instead.
> >
> >
> Right there wasn't pg_database_ownercheck at the time I was writing it,
> fixed
>
> >The rest needs to be updated for roles, but looks like it should be
> >pretty easy to do. Much of it just needs to be repatched, the parts
> >that do need to be changed look to be pretty simple changes.
> >
> >
> Done.
>
> >I believe the use of SessionUserId is probably correct in this patch.
> >This does mean that this patch will only be for canlogin roles, but that
> >seems like it's probably correct. Handling roles w/ members would
> >require much more thought.
> >
> >
> I don't think that having max connection for roles w/ members is doable
> because you can have 5 roles which has 1 user as member and each role
> has different number of max conections and there is no right way to
> decide what to do.
>
>
> New version which works with roles is attached (diffed against cvs),
> everything else is mostly same.
> I also had to readd roleid to flatfiles because I need it in
> InitProcess() function.
>
> --
> Regards
> Petr Jelinek (PJMODOS)
>
>
> Index: src/backend/commands/dbcommands.c
> ===
> RCS file: /projects/cvsroot/pgsql/src/backend/commands/dbcommands.c,v
> retrieving revision 1.164
> diff -c -r1.164 dbcommands.c
> *** src/backend/commands/dbcommands.c 30 Jun 2005 00:00:50 - 1.164
> --- src/backend/commands/dbcommands.c 3 Jul 2005 22:47:39 -
> ***
> *** 53,60
>
> /* non-export function prototypes */
> static bool get_db_info(const char *name, Oid *dbIdP, Oid *ownerIdP,
> ! int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP,
> ! Oid *dbLastSysOidP,
> TransactionId *dbVacuumXidP, TransactionId
> *dbFrozenXidP,
> Oid *dbTablespace);
> static bool have_createdb_privilege(void);
> --- 53,60
>
> /* non-export function prototypes */
> static bool get_db_info(const char *name, Oid *dbIdP, Oid *ownerIdP,
> ! int *encodingP, int *dbMaxConnP, bool *dbIsTemplateP,
> ! bool *dbAllowConnP, Oid *dbLastSysOidP,
> TransactionId *dbVacuumXidP, TransactionId
> *dbFrozenXidP,
> Oid *dbTablespace);
> static bool have_createdb_privilege(void);
> ***
> *** 74,79
> --- 74,80
> int src_encoding;
> boolsrc_istemplate;
> boolsrc_allowconn;
> + int src_maxconn;
> Oid src_lastsysoid;
> TransactionId src_vacuumxid;
> TransactionId src_frozenxid;
> ***
> *** 91,100
> --- 92,103
> DefElem*downer = NULL;
> DefElem*dtemplate = NULL;
> DefElem*dencoding = NULL;
> + DefElem*dmaxconn = NULL;
> char *dbname = stmt->dbname;
> char *dbowner = NULL;
> const char *dbtemplate = NULL;
> int encoding = -1;
> + int dbmaxconn = -1;
>
> #ifndef WIN32
> charbuf[2 * MAXPGPATH + 100];
> ***
> *** 140,145
> --- 143,156
>errmsg("conflicting or
> redundant options")));
> dencoding = defel;
> }
> + else if (strcmp(defel->defname, "maxconnections") == 0)
> + {
> + if (dmaxconn)
> + ereport(ERROR,
> + (errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("conflicting or
> redundant options")));
> + dmaxconn = defel;
> + }
> else if (strcmp(defel->defname, "location") == 0)
> {
> ereport(WARNING,
> ***
> *** 185,190
> --- 196,203
> elog(ERROR, "unrecognized node type: %d",
>nodeTag(dencoding->arg));
> }
> + if (dmaxconn && dmaxconn->arg)
> + dbmaxconn = intVal(dmaxconn->arg);
>
> /* obtain OID of proposed owner */
> if (dbowner)
> ***
> *** 218,224
>* idea, so accept possibility of race to create. We will check again
>* after we grab the exclusive lock.
>*/
> ! if (get_db_
Re: [PATCHES] [HACKERS] regressin failure on latest CVS
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:[EMAIL PROTECTED] > > > 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:[EMAIL PROTECTED] > > > > > 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 [EMAIL PROTECTED] 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 - 1.145 > --- src/backend/utils/adt/timestamp.c 23 Jul
Re: [PATCHES] [HACKERS] regressin failure on latest CVS
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:[EMAIL PROTECTED]
> > 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:[EMAIL PROTECTED]
> > > > 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 [EMAIL PROTECTED] 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 - 1.145
--- src/backend/utils/adt/timestamp.c 23 Jul 2005 19:34:39 -
***
*** 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
Re: [PATCHES] [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:[EMAIL PROTECTED] > 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:[EMAIL PROTECTED] > > > 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 [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] [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:[EMAIL PROTECTED] > > 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 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 - 1.145 --- src/backend/utils/adt/timestamp.c 23 Jul 2005 18:45:56 - *** *** 2309,2327 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 --- 2309,2328 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 */ #ifdef HAVE_INT64_TIMESTAMP ! result->time += (day_remainder + month_remainder * DAYS_PER_MONTH - ! (int)(month_remainder * DAYS_PER_MONTH)) * USECS_PER_DAY; #else ! result->time += (day_remainder + month_remainder * DAYS_PER_MONTH - ! (int)(month_remainder * DAYS_PER_MONTH)) * SECS_PER_DAY; result->time = JROUND(result->time); #endif ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] O_DIRECT for WAL writes
I have modified and attached your patch for your review. I didn't see
any value to adding new fsync_method values because, to me, O_DIRECT is
basically just like O_SYNC except it doesn't keep a copy of the buffer
in the kernel cache. If you are doing fsync(), I don't see how O_DIRECT
makes any sense because O_DIRECT is writing to disk on every write, and
then what is the fsync() actually doing. This might explain why your
fsync/direct and open/direct performance numbers are almost identical.
Basically, if you are going to use O_DIRECT, why not use open_sync.
What I did was to add O_DIRECT unconditionally for all uses of O_SYNC
and O_DSYNC, so it is automatically used in those cases. And of course,
if your operating system doens't support O_DIRECT, it isn't used.
With your posted performance numbers, perhaps we should favor
fsync_method O_SYNC on platforms that have O_DIRECT even if we don't
support OPEN_DATASYNC, but I bet most platforms that have O_DIRECT also
have O_DATASYNC. Perhaps some folks can run testes once the patch is
applied.
---
ITAGAKI Takahiro wrote:
> Tom Lane <[EMAIL PROTECTED]> wrote:
>
> > Yeah, this is about what I was afraid of: if you're actually fsyncing
> > then you get at best one commit per disk revolution, and the negotiation
> > with the OS is down in the noise.
>
> If we disable writeback-cache and use open_sync, the per-page writing
> behavior in WAL module will show up as bad result. O_DIRECT is similar
> to O_DSYNC (at least on linux), so that the benefit of it will disappear
> behind the slow disk revolution.
>
> In the current source, WAL is written as:
> for (i = 0; i < N; i++) { write(&buffers[i], BLCKSZ); }
> Is this intentional? Can we rewrite it as follows?
>write(&buffers[0], N * BLCKSZ);
>
> In order to achieve it, I wrote a 'gather-write' patch (xlog.gw.diff).
> Aside from this, I'll also send the fixed direct io patch (xlog.dio.diff).
> These two patches are independent, so they can be applied either or both.
>
>
> I tested them on my machine and the results as follows. It shows that
> direct-io and gather-write is the best choice when writeback-cache is off.
> Are these two patches worth trying if they are used together?
>
>
> | writeback | fsync= | fdata | open_ | fsync_ | open_
> patch | cache | false | sync | sync | direct | direct
> +---++---+---++-
> direct io | off | 124.2 | 105.7 | 48.3 | 48.3 | 48.2
> direct io | on| 129.1 | 112.3 | 114.1 | 142.9 | 144.5
> gather-write| off | 124.3 | 108.7 | 105.4 | (N/A) | (N/A)
> both| off | 131.5 | 115.5 | 114.4 | 145.4 | 145.2
>
> - 20runs * pgbench -s 100 -c 50 -t 200
>- with tuning (wal_buffers=64, commit_delay=500, checkpoint_segments=8)
> - using 2 ATA disks:
>- hda(reiserfs) includes system and wal.
>- hdc(jfs) includes database files. writeback-cache is always on.
>
> ---
> ITAGAKI Takahiro
> NTT Cyber Space Laboratories
>
[ Attachment, skipping... ]
[ Attachment, skipping... ]
>
> ---(end of broadcast)---
> TIP 5: Have you checked our extensive FAQ?
>
>http://www.postgresql.org/docs/faq
--
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/access/transam/xlog.c
===
RCS file: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.210
diff -c -c -r1.210 xlog.c
*** src/backend/access/transam/xlog.c 23 Jul 2005 15:31:16 - 1.210
--- src/backend/access/transam/xlog.c 23 Jul 2005 16:09:12 -
***
*** 48,77
/*
* This chunk of hackery attempts to determine which file sync methods
* are available on the current platform, and to choose an appropriate
* default method.We assume that fsync() is always available, and that
* configure determined whether fdatasync() is.
*/
#if defined(O_SYNC)
! #define OPEN_SYNC_FLAGO_SYNC
#else
#if defined(O_FSYNC)
! #define OPEN_SYNC_FLAGO_FSYNC
#endif
#endif
#if defined(O_DSYNC)
#if defined(OPEN_SYNC_FLAG)
! #if O_DSYNC != OPEN_SYNC_FLAG
! #define OPEN_DATASYNC_FLAGO_DSYNC
#endif
#else /* !defined(OPEN_SYNC_FLAG) */
/* Win32 only has O_DSYNC */
! #define OPEN_DATASYNC_FLAGO_DSYNC
#endif
#endif
#if defined(OPEN_DATASYNC_FLAG)
#define DEFAULT_SYNC_METHOD_STR "open_datasync"
#define DEFAULT_SYNC_METHOD SYNC_METHOD_OPEN
--- 48,114
/*
+ *Becauase O_DIRECT bypasses the kernel buffers, and
Re: [PATCHES] [HACKERS] regressin failure on latest CVS
Yes, we have seen those stat tests fail randomly. We are working on a solution. --- ohp@pyrenet.fr wrote: > I think the patch is ok now, intervall is not failing anymore as of > 18:50 CET. > > However stats fails. > regression.diffs: > > *** ./expected/stats.out Sat Jul 23 17:18:20 2005 > --- ./results/stats.out Sat Jul 23 18:55:17 2005 > *** > *** 53,59 >WHERE st.relname='tenk2' AND cl.relname='tenk2'; >?column? | ?column? | ?column? | ?column? > --+--+--+-- > ! t| t| t| t > (1 row) > > SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks + cl.relpages, > --- 53,59 >WHERE st.relname='tenk2' AND cl.relname='tenk2'; >?column? | ?column? | ?column? | ?column? > --+--+--+-- > ! f| f| t| t > (1 row) > > SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks + cl.relpages, > *** > *** 62,68 >WHERE st.relname='tenk2' AND cl.relname='tenk2'; >?column? | ?column? > --+-- > ! t| t > (1 row) > > -- End of Stats Test > --- 62,68 >WHERE st.relname='tenk2' AND cl.relname='tenk2'; >?column? | ?column? > --+-- > ! f| t > (1 row) > > -- End of Stats Test > > == > > On Sat, 23 Jul 2005, Bruce Momjian wrote: > > > Date: Sat, 23 Jul 2005 11:36:43 -0400 (EDT) > > From: Bruce Momjian > > To: ohp@pyrenet.fr > > Cc: Rocco Altier <[EMAIL PROTECTED]>, > > Michael Glaesemann <[EMAIL PROTECTED]>, pgsql-patches@postgresql.org, > > pgsql-hackers@postgresql.org > > Subject: Re: [HACKERS] regressin failure on latest CVS > > > > ohp@pyrenet.fr wrote: > > > I just checked latest CVS (5 mn ago) the problem is still the same, > > > BTW, this is on Unixware 714 and no --enable-integer-datetime > > > > Do you have the latest patch included int that version of CVS? > > Anonymous CVS has a delay, and what was the problem you were seeing, the > > rounding or the day - 1 result? > > > > --- > > > > > > > > > > Regards > > > On Sat, 23 Jul 2005, Rocco Altier wrote: > > > > > > > Date: Sat, 23 Jul 2005 11:15:44 -0400 > > > > From: Rocco Altier <[EMAIL PROTECTED]> > > > > To: Bruce Momjian > > > > Cc: Michael Glaesemann <[EMAIL PROTECTED]>, > > > > pgsql-patches@postgresql.org, > > > > pgsql-hackers@postgresql.org, ohp@pyrenet.fr > > > > Subject: RE: [HACKERS] regressin failure on latest CVS > > > > > > > > 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:[EMAIL PROTECTED] > > > > > 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. > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > -- > > > Olivier PRENANT Tel: +33-5-61-50-97-00 (Work) > > > 15, Chemin des Monges+33-5-61-50-97-01 (Fax) > > > 31190 AUTER
Re: [PATCHES] [HACKERS] regressin failure on latest CVS
I think the patch is ok now, intervall is not failing anymore as of 18:50 CET. However stats fails. regression.diffs: *** ./expected/stats.outSat Jul 23 17:18:20 2005 --- ./results/stats.out Sat Jul 23 18:55:17 2005 *** *** 53,59 WHERE st.relname='tenk2' AND cl.relname='tenk2'; ?column? | ?column? | ?column? | ?column? --+--+--+-- ! t| t| t| t (1 row) SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks + cl.relpages, --- 53,59 WHERE st.relname='tenk2' AND cl.relname='tenk2'; ?column? | ?column? | ?column? | ?column? --+--+--+-- ! f| f| t| t (1 row) SELECT st.heap_blks_read + st.heap_blks_hit >= pr.heap_blks + cl.relpages, *** *** 62,68 WHERE st.relname='tenk2' AND cl.relname='tenk2'; ?column? | ?column? --+-- ! t| t (1 row) -- End of Stats Test --- 62,68 WHERE st.relname='tenk2' AND cl.relname='tenk2'; ?column? | ?column? --+-- ! f| t (1 row) -- End of Stats Test == On Sat, 23 Jul 2005, Bruce Momjian wrote: > Date: Sat, 23 Jul 2005 11:36:43 -0400 (EDT) > From: Bruce Momjian > To: ohp@pyrenet.fr > Cc: Rocco Altier <[EMAIL PROTECTED]>, > Michael Glaesemann <[EMAIL PROTECTED]>, pgsql-patches@postgresql.org, > pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] regressin failure on latest CVS > > ohp@pyrenet.fr wrote: > > I just checked latest CVS (5 mn ago) the problem is still the same, > > BTW, this is on Unixware 714 and no --enable-integer-datetime > > Do you have the latest patch included int that version of CVS? > Anonymous CVS has a delay, and what was the problem you were seeing, the > rounding or the day - 1 result? > > --- > > > > > > Regards > > On Sat, 23 Jul 2005, Rocco Altier wrote: > > > > > Date: Sat, 23 Jul 2005 11:15:44 -0400 > > > From: Rocco Altier <[EMAIL PROTECTED]> > > > To: Bruce Momjian > > > Cc: Michael Glaesemann <[EMAIL PROTECTED]>, pgsql-patches@postgresql.org, > > > pgsql-hackers@postgresql.org, ohp@pyrenet.fr > > > Subject: RE: [HACKERS] regressin failure on latest CVS > > > > > > 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:[EMAIL PROTECTED] > > > > 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. > > > > > > > > -- > > > > > > > > > > > > > -- > > Olivier PRENANT Tel: +33-5-61-50-97-00 (Work) > > 15, Chemin des Monges+33-5-61-50-97-01 (Fax) > > 31190 AUTERIVE +33-6-07-63-80-64 (GSM) > > FRANCE Email: ohp@pyrenet.fr > > -- > > Make your life a dream, make your dream a reality. (St Exupery) > > > > ---(end of broadcast)--- > > TIP 6: explain analyze is your friend > > > > -- Olivier PRENANT Tel: +33-5-61-50-97-00 (Work) 15, Chemin des
Re: [PATCHES] [HACKERS] regressin failure on latest CVS
On Sat, 23 Jul 2005, Bruce Momjian wrote: > Date: Sat, 23 Jul 2005 11:36:43 -0400 (EDT) > From: Bruce Momjian > To: ohp@pyrenet.fr > Cc: Rocco Altier <[EMAIL PROTECTED]>, > Michael Glaesemann <[EMAIL PROTECTED]>, pgsql-patches@postgresql.org, > pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] regressin failure on latest CVS > > ohp@pyrenet.fr wrote: > > I just checked latest CVS (5 mn ago) the problem is still the same, > > BTW, this is on Unixware 714 and no --enable-integer-datetime > > Do you have the latest patch included int that version of CVS? > Anonymous CVS has a delay, and what was the problem you were seeing, the > rounding or the day - 1 result? > I was seeing (and still see) the day -1 result. However, if I ./configure --with-integer-datetimes I see the rounding of the day. > --- > > > > > > Regards > > On Sat, 23 Jul 2005, Rocco Altier wrote: > > > > > Date: Sat, 23 Jul 2005 11:15:44 -0400 > > > From: Rocco Altier <[EMAIL PROTECTED]> > > > To: Bruce Momjian > > > Cc: Michael Glaesemann <[EMAIL PROTECTED]>, pgsql-patches@postgresql.org, > > > pgsql-hackers@postgresql.org, ohp@pyrenet.fr > > > Subject: RE: [HACKERS] regressin failure on latest CVS > > > > > > 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:[EMAIL PROTECTED] > > > > 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. > > > > > > > > -- > > > > > > > > > > > > > -- > > Olivier PRENANT Tel: +33-5-61-50-97-00 (Work) > > 15, Chemin des Monges+33-5-61-50-97-01 (Fax) > > 31190 AUTERIVE +33-6-07-63-80-64 (GSM) > > FRANCE Email: ohp@pyrenet.fr > > -- > > Make your life a dream, make your dream a reality. (St Exupery) > > > > ---(end of broadcast)--- > > TIP 6: explain analyze is your friend > > > > -- Olivier PRENANT Tel: +33-5-61-50-97-00 (Work) 15, Chemin des Monges+33-5-61-50-97-01 (Fax) 31190 AUTERIVE +33-6-07-63-80-64 (GSM) FRANCE Email: ohp@pyrenet.fr -- Make your life a dream, make your dream a reality. (St Exupery) ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] regressin failure on latest CVS
ohp@pyrenet.fr wrote: > I just checked latest CVS (5 mn ago) the problem is still the same, > BTW, this is on Unixware 714 and no --enable-integer-datetime Do you have the latest patch included int that version of CVS? Anonymous CVS has a delay, and what was the problem you were seeing, the rounding or the day - 1 result? --- > > Regards > On Sat, 23 Jul 2005, Rocco Altier wrote: > > > Date: Sat, 23 Jul 2005 11:15:44 -0400 > > From: Rocco Altier <[EMAIL PROTECTED]> > > To: Bruce Momjian > > Cc: Michael Glaesemann <[EMAIL PROTECTED]>, pgsql-patches@postgresql.org, > > pgsql-hackers@postgresql.org, ohp@pyrenet.fr > > Subject: RE: [HACKERS] regressin failure on latest CVS > > > > 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:[EMAIL PROTECTED] > > > 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. > > > > > > -- > > > > > > > > -- > Olivier PRENANT Tel: +33-5-61-50-97-00 (Work) > 15, Chemin des Monges+33-5-61-50-97-01 (Fax) > 31190 AUTERIVE +33-6-07-63-80-64 (GSM) > FRANCE Email: ohp@pyrenet.fr > -- > Make your life a dream, make your dream a reality. (St Exupery) > > ---(end of broadcast)--- > TIP 6: explain analyze is your friend > -- 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 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] regressin failure on latest CVS
I just checked latest CVS (5 mn ago) the problem is still the same, BTW, this is on Unixware 714 and no --enable-integer-datetime Regards On Sat, 23 Jul 2005, Rocco Altier wrote: > Date: Sat, 23 Jul 2005 11:15:44 -0400 > From: Rocco Altier <[EMAIL PROTECTED]> > To: Bruce Momjian > Cc: Michael Glaesemann <[EMAIL PROTECTED]>, pgsql-patches@postgresql.org, > pgsql-hackers@postgresql.org, ohp@pyrenet.fr > Subject: RE: [HACKERS] regressin failure on latest CVS > > 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:[EMAIL PROTECTED] > > 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. > > > > -- > > > -- Olivier PRENANT Tel: +33-5-61-50-97-00 (Work) 15, Chemin des Monges+33-5-61-50-97-01 (Fax) 31190 AUTERIVE +33-6-07-63-80-64 (GSM) FRANCE Email: ohp@pyrenet.fr -- Make your life a dream, make your dream a reality. (St Exupery) ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] regressin failure on latest CVS
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:[EMAIL PROTECTED] > 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 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [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. --- > Thanks, > -rocco > > > -Original Message- > > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED] On Behalf Of Bruce Momjian > > Sent: Friday, July 22, 2005 10:41 AM > > To: Michael Glaesemann > > Cc: ohp@pyrenet.fr; pgsql-hackers@postgresql.org > > Subject: Re: [HACKERS] regressin failure on latest CVS > > > > > > Michael Glaesemann wrote: > > > > > > On Jul 22, 2005, at 6:28 PM, ohp@pyrenet.fr wrote: > > > > > > > I tried the latest cvs this morning (07/22 11:00 CET) > > > > and interval test fails. > > > > Here's the regression.diffs. > > > > > > > > > > > *** ./expected/interval.outFri Jul 22 10:32:21 2005 > > > > --- ./results/interval.outFri Jul 22 11:07:54 2005 > > > > *** > > > > *** 217,224 > > > > -- updating pg_aggregate.agginitval > > > > 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 > > > > --- 217,224 > > > > -- updating pg_aggregate.agginitval > > > > select avg(f1) from interval_tbl; > > > > avg > > > > ! > > > > ! @ 4 years 1 mon 9 days 4 hours 18 mins 23 secs > > > > (1 row) > > > > > > > > -- test long interval input > > > > > > Could you provide platform information? Did you build with > > --enable- > > > integer-datetimes? Looking at the buildfarm, kookaburra > > (AIX 5.2) is > > > also failing the interval test at the same point, but the > > result is > > > different. > > > > Interesting. I don't see the error with our without > > --enable-integer-datetimes. I even tried changing my > > timezone to Paris > > time and still could not reproduce the failure. > > > > On the AIX problem below, we are going to get rounding issues. > > > > -- > > - > > > > > > > http://pgbuildfarm.org/cgi-bin/show_history.pl?nm=kookaburra&br=HEAD > > > > > > == pgsql.36852/src/test/regress/regression.diffs > > > === > > > *** ./expected/interval.out Fri Jul 22 01:25:05 2005 > > > --- ./results/interval.out Fri Jul 22 01:34:20 2005 > > > *** > > > *** 217,224 > > >-- updating pg_aggregate.agginitval > > >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 > > > --- 217,224 > > >-- updating pg_aggregate.agginitval > > >select avg(f1) from interval_tbl; > > >avg > > > ! > > > ! @ 4 years 1 mon 10 days 4 hours 18 mins 22.99 secs > > >(1 row) > > > > > > > > > Michael Glaesemann > > > grzm myrealbox com > > > > > > > > > > > > ---(end of > > broadcast)--- > > > TIP 1: if posting/reading through Usenet, please send an appropriate > > >subscribe-nomail command to [EMAIL PROTECTED] > > 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 > > > > ---(end of > > broadcast)--- > > TIP 4: Have you searched our list archives? > > >http://archives.postgresql.org Content-Description: timestamp.patch [ Attachment, skipping... ] > > ---(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
Re: [PATCHES] [HACKERS] Timezone bugs
Andrew pointed out that the current fix didn't handle dates that were
near daylight savings time boudaries. This handles it properly, e.g.
test=> select '2005-04-03 04:00:00'::timestamp at time zone
'America/Los_Angeles';
timezone
2005-04-03 07:00:00-04
(1 row)
Patch attached and applied. The new fix is cleaner too.
---
pgman wrote:
>
> OK, tricky, but fixed --- patch attached and applied, with documentation
> updates. Here is the test query:
>
> test=> select (CURRENT_DATE + '05:00'::time)::timestamp at time zone
> 'Canada/Pacific';
> timezone
>
>2005-07-22 08:00:00-04
> (1 row)
>
> I tested a bunch of others too, like:
>
> test=> select ('2005-07-20 00:00:00'::timestamp without time zone) at
> time zone 'Europe/Paris';
> timezone
>
>2005-07-19 18:00:00-04
> (1 row)
>
> and tested that for UTC also.
>
> It was hard to figure out how to cleanly adjust the time zone. I added
> some comments explaining the process.
>
> ---
>
> Andrew - Supernews wrote:
> > On 2005-07-22, Bruce Momjian wrote:
> > >>
> > >> select (CURRENT_DATE + '05:00'::time)::timestamp
> > >>at time zone 'Canada/Pacific';
> > >> timezone
> > >>
> > >> 2005-07-19 22:00:00+00
> > >> (1 row)
> > >>
> > > What is happening here is that 2005-07-20 05:00:00 is being cast back 7
> > > hours (Canada/Pacific offset), and that is 22:00 of the previous day.
> >
> > Which is of course completely wrong.
> >
> > Let's look at what should happen:
> >
> > (date + time) = timestamp without time zone
> >
> > '2005-07-20' + '05:00' = '2005-07-20 05:00:00'::timestamp
> >
> > (timestamp without time zone) AT TIME ZONE 'zone'
> >
> > When AT TIME ZONE is applied to a timestamp without time zone, it is
> > supposed to keep the _same_ calendar time and return a result of type
> > timestamp with time zone designating the absolute time. So in this case,
> > we expect the following to happen:
> >
> > '2005-07-20 05:00:00' (original timestamp)
> > -> '2005-07-20 05:00:00-0700' (same calendar time in new zone)
> > -> '2005-07-20 12:00:00+' (convert to client timezone (UTC))
> >
> > So the conversion is being done backwards, resulting in the wrong result.
> >
> > --
> > Andrew, Supernews
--
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/date.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/date.c,v
retrieving revision 1.118
diff -c -c -r1.118 date.c
*** src/backend/utils/adt/date.c22 Jul 2005 05:03:09 - 1.118
--- src/backend/utils/adt/date.c23 Jul 2005 14:23:14 -
***
*** 301,307
tm->tm_hour = 0;
tm->tm_min = 0;
tm->tm_sec = 0;
! tz = DetermineLocalTimeZone(tm);
#ifdef HAVE_INT64_TIMESTAMP
result = dateVal * USECS_PER_DAY + tz * USECS_PER_SEC;
--- 301,307
tm->tm_hour = 0;
tm->tm_min = 0;
tm->tm_sec = 0;
! tz = DetermineTimeZoneOffset(tm, global_timezone);
#ifdef HAVE_INT64_TIMESTAMP
result = dateVal * USECS_PER_DAY + tz * USECS_PER_SEC;
***
*** 2231,2237
GetCurrentDateTime(tm);
time2tm(time, tm, &fsec);
! tz = DetermineLocalTimeZone(tm);
result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
--- 2231,2237
GetCurrentDateTime(tm);
time2tm(time, tm, &fsec);
! tz = DetermineTimeZoneOffset(tm, global_timezone);
result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
Index: src/backend/utils/adt/datetime.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.156
diff -c -c -r1.156 datetime.c
*** src/backend/utils/adt/datetime.c22 Jul 2005 03:46:33 - 1.156
--- src/backend/utils/adt/datetime.c23 Jul 2005 14:23:15 -
***
*** 1612,1618
if (fmask & DTK_M(DTZMOD))
return DTERR_BAD_FORMAT;
! *tzp = DetermineLocalTimeZone(tm);
}
}
--- 1612,1618
if (fmask & DTK_M(DTZMOD))
return DTERR_BAD_FORMAT;
! *tzp = DetermineTimeZoneOffset(tm, global_timezone);
