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: [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 =
