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 pgman@candle.pha.pa.us 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);
}
}
***
*** 1620,1629
}
! /* DetermineLocalTimeZone()
*
*
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=kookaburrabr=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 + 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
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
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 pgman@candle.pha.pa.us 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
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 pgman@candle.pha.pa.us 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
On Sat, 23 Jul 2005, Bruce Momjian wrote: Date: Sat, 23 Jul 2005 11:36:43 -0400 (EDT) From: Bruce Momjian pgman@candle.pha.pa.us 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 pgman@candle.pha.pa.us 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
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 pgman@candle.pha.pa.us 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 pgman@candle.pha.pa.us 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
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 pgman@candle.pha.pa.us 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 pgman@candle.pha.pa.us 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
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 because we never
+ *read those buffers except during
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: [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
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,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
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 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,2300
{
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor =
[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
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
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: [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_remainder * USECS_PER_DAY;
#else
[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
