Re: [HACKERS] float8 regression failure (HEAD, cygwin)
On 01/08/06, Andrew Dunstan [EMAIL PROTECTED] wrote: Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Would this do the trick? I think Bruce changed the call convention for run_diff ... are you looking at CVS tip? Otherwise it looks reasonable. You're right. I had forgotten to do a cvs update. Fixed and committed. The cassowary farm member went green, for the first time in history :) Thanks guys, Adrian Maier ---(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: [HACKERS] float8 regression failure (HEAD, cygwin)
On 20/07/06, Tom Lane [EMAIL PROTECTED] wrote: Reini Urban [EMAIL PROTECTED] writes: BTW: HAVE_LONG_LONG_INT_64 is defined, so INT64_IS_BUSTED is defined also. You sure? INT64_IS_BUSTED should *not* be set in that case --- it's only supposed to be set if we couldn't find any 64-bit-int type at all. As for the regression test failure, it's odd because it looks to me that the actual test output is an exact match to the default float8.out file. I'm not sure why pg_regress chose to report a diff against float8-small-is-zero.out instead. This may be another teething pain of the new pg_regress-in-C code --- could you trace through it and see what's happening? Apparently the regression test is comparing the results/float8.out with expected/float8-small-is-zero.out because of the following line in src/test/regress/resultmap : float8/i.86-pc-cygwin=float8-small-is-zero I've changed that line to : float8/i.86-pc-cygwin=float8 and the regression test ended successfully : All 100 tests passed. I don't know why there are several expected results for the float8 test, depending on the platform. Is the modification ok? I've attached the patch, and cc'ed to pgsql-patches. Cheers, Adrian Maier patch_float8.diff Description: Binary data ---(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] float8 regression failure (HEAD, cygwin)
Adrian Maier wrote: On 20/07/06, Tom Lane [EMAIL PROTECTED] wrote: Reini Urban [EMAIL PROTECTED] writes: BTW: HAVE_LONG_LONG_INT_64 is defined, so INT64_IS_BUSTED is defined also. You sure? INT64_IS_BUSTED should *not* be set in that case --- it's only supposed to be set if we couldn't find any 64-bit-int type at all. As for the regression test failure, it's odd because it looks to me that the actual test output is an exact match to the default float8.out file. I'm not sure why pg_regress chose to report a diff against float8-small-is-zero.out instead. This may be another teething pain of the new pg_regress-in-C code --- could you trace through it and see what's happening? Apparently the regression test is comparing the results/float8.out with expected/float8-small-is-zero.out because of the following line in src/test/regress/resultmap : float8/i.86-pc-cygwin=float8-small-is-zero I've changed that line to : float8/i.86-pc-cygwin=float8 and the regression test ended successfully : All 100 tests passed. I don't know why there are several expected results for the float8 test, depending on the platform. Is the modification ok? I've attached the patch, and cc'ed to pgsql-patches. The problem with this is that we have another Cygwin member on buildfarm which passes the tests happily, and will thus presumably fail if we make this patch. You are running Cygwin 1.5.21 and the other buildfarm member is running 1.5.19, so that is possibly the difference. Maybe we need to abandon trying to map float8 results exactly in the resultmap file, and just let pg_regress pick the best fit as we do with some other tests. cheers andrew ---(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: [HACKERS] float8 regression failure (HEAD, cygwin)
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Maybe we need to abandon trying to map float8 results exactly in the resultmap file, and just let pg_regress pick the best fit as we do with some other tests. I thought about that too but it seems a very bad idea. small-is-zero is distinctly less correct than the regular output, and I don't think we want pg_regress to be blindly accepting it as OK on any platform. Perhaps we could stick a version check into the resultmap lookup? It'd likely have been painful on the shell script implementation but now that the code is in C I think we have lots of flexibility. There's no need to feel bound by the historical resultmap format. However this is all premature unless we can verify that cgywin's strtod() complains about float underflow after version so-and-so. Do they publish a detailed change log? Yes, good points. One other thought I had was that we could have pg_regress always allow a fallback to the canonical result file. So in this case it would try float8-small-is-zero.out and float8-small-is-zero_1.out and finally float8.out (the first would succeed on eel, the last on cassowary, I am speculating). In general this would allow a configuration to become more correct painlessly. cheers andrew ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] float8 regression failure (HEAD, cygwin)
On 01/08/06, Andrew Dunstan [EMAIL PROTECTED] wrote: Adrian Maier wrote: On 20/07/06, Tom Lane [EMAIL PROTECTED] wrote: Apparently the regression test is comparing the results/float8.out with expected/float8-small-is-zero.out because of the following line in src/test/regress/resultmap : float8/i.86-pc-cygwin=float8-small-is-zero I've changed that line to : float8/i.86-pc-cygwin=float8 and the regression test ended successfully : All 100 tests passed. I don't know why there are several expected results for the float8 test, depending on the platform. Is the modification ok? I've attached the patch, and cc'ed to pgsql-patches. The problem with this is that we have another Cygwin member on buildfarm which passes the tests happily, and will thus presumably fail if we make this patch. You are running Cygwin 1.5.21 and the other buildfarm member is running 1.5.19, so that is possibly the difference. This is indeed a problem. It would be difficult or even impossible to use different expected results for different versions of cygwin. Maybe we need to abandon trying to map float8 results exactly in the resultmap file, and just let pg_regress pick the best fit as we do with some other tests. Oh, is it possible to do that? That sounds great. Which other tests work like that? Cheers, Adrian Maier ---(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] float8 regression failure (HEAD, cygwin)
Andrew Dunstan [EMAIL PROTECTED] writes: Maybe we need to abandon trying to map float8 results exactly in the resultmap file, and just let pg_regress pick the best fit as we do with some other tests. I thought about that too but it seems a very bad idea. small-is-zero is distinctly less correct than the regular output, and I don't think we want pg_regress to be blindly accepting it as OK on any platform. Perhaps we could stick a version check into the resultmap lookup? It'd likely have been painful on the shell script implementation but now that the code is in C I think we have lots of flexibility. There's no need to feel bound by the historical resultmap format. However this is all premature unless we can verify that cgywin's strtod() complains about float underflow after version so-and-so. Do they publish a detailed change log? regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] float8 regression failure (HEAD, cygwin)
On 01/08/06, Tom Lane [EMAIL PROTECTED] wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Maybe we need to abandon trying to map float8 results exactly in the resultmap file, and just let pg_regress pick the best fit as we do with some other tests. I thought about that too but it seems a very bad idea. small-is-zero is distinctly less correct than the regular output, and I don't think we want pg_regress to be blindly accepting it as OK on any platform. Perhaps we could stick a version check into the resultmap lookup? It'd likely have been painful on the shell script implementation but now that the code is in C I think we have lots of flexibility. There's no need to feel bound by the historical resultmap format. However this is all premature unless we can verify that cgywin's strtod() complains about float underflow after version so-and-so. Do they publish a detailed change log? There are links to the last few releases on their home page http://www.cygwin.com , in the News section. -- Adrian Maier ---(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] float8 regression failure (HEAD, cygwin)
Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: I thought about that too but it seems a very bad idea. small-is-zero is distinctly less correct than the regular output, and I don't think we want pg_regress to be blindly accepting it as OK on any platform. Yes, good points. One other thought I had was that we could have pg_regress always allow a fallback to the canonical result file. Hm, that's a good thought. Want to see how painful it is to code? One thing I realized while looking at resultmap just now is that the handmade regex matcher code I wrote for pg_regress.c (lifted from the backend's LIKE algorithm) doesn't handle bracketed character classes, but we've got one of those in resultmap: float8/i.86-.*-freebsd[234]=float8-small-is-zero Dunno how I missed that :-(. So presumably HEAD would be failing right now on freebsd-before-5.0, if we had any of 'em in the buildfarm. Adding char-class code to the matcher wouldn't be too hard, but it probably wouldn't solve the cygwin problem, and as I mentioned I feel zero allegiance to the historical format of resultmap anyway --- it was designed the way it is purely because it was fairly easy to handle in a few lines of shell script. If we used your idea of always allowing a fallback to the canonical file, then we could just make the freebsd line float8/i.86-.*-freebsd=float8-small-is-zero and it would do the right thing. Alternatively we could do something more adventurous with matching to OS versions --- I'm envisioning something roughly like float8/i.86-.*-freebsd5=float8-small-is-zero float8/i.86-pc-cygwin5.0.21=float8-small-is-zero This would be more flexible in the long run, but might be overkill. The fallback idea sounds like a reasonable compromise ... regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] float8 regression failure (HEAD, cygwin)
Tom Lane wrote:
Andrew Dunstan [EMAIL PROTECTED] writes:
One other thought I had was that we could have
pg_regress always allow a fallback to the canonical result file.
Hm, that's a good thought. Want to see how painful it is to code?
Would this do the trick?
cheers
andrew
Index: pg_regress.c
===
RCS file: /cvsroot/pgsql/src/test/regress/pg_regress.c,v
retrieving revision 1.16
diff -c -r1.16 pg_regress.c
*** pg_regress.c 27 Jul 2006 15:37:19 - 1.16
--- pg_regress.c 1 Aug 2006 14:04:20 -
***
*** 914,919
--- 914,952
}
}
+ /*
+ * fall back on the canonical results file if we haven't tried it yet
+ * and haven't found a complete match yet.
+ */
+
+ if (strcmp(expectname, testname) != 0)
+ {
+ snprintf(expectfile, sizeof(expectfile), %s/expected/%s.out,
+ inputdir, testname, i);
+ if (!file_exists(expectfile))
+ continue;
+
+ snprintf(cmd, sizeof(cmd),
+ SYSTEMQUOTE diff %s \%s\ \%s\ \%s\ SYSTEMQUOTE,
+ basic_diff_opts, expectfile, resultsfile, diff);
+ run_diff(cmd);
+
+ if (file_size(diff) == 0)
+ {
+ /* No diff = no changes = good */
+ unlink(diff);
+ return false;
+ }
+
+ l = file_line_count(diff);
+ if (l best_line_count)
+ {
+ /* This diff was a better match than the last one */
+ best_line_count = l;
+ strcpy(best_expect_file, expectfile);
+ }
+ }
+
/*
* Use the best comparison file to generate the pretty diff, which
* we append to the diffs summary file.
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] float8 regression failure (HEAD, cygwin)
Andrew Dunstan [EMAIL PROTECTED] writes: Would this do the trick? I think Bruce changed the call convention for run_diff ... are you looking at CVS tip? Otherwise it looks reasonable. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] float8 regression failure (HEAD, cygwin)
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Would this do the trick? I think Bruce changed the call convention for run_diff ... are you looking at CVS tip? Otherwise it looks reasonable. You're right. I had forgotten to do a cvs update. Fixed and committed. cheers andrew ---(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: [HACKERS] float8 regression failure (HEAD, cygwin)
Adrian Maier schrieb:
Hello,
While setting up a buildfarm installation for cygwin, I've
uncountered the following
regression failure :
float8 ... FAILED
== pgsql.3132/src/test/regress/regression.diffs
*** ./expected/float8-small-is-zero.outTue Jul 18 09:24:52 2006
--- ./results/float8.outTue Jul 18 09:53:42 2006
***
*** 13,29
SELECT '-10e400'::float8;
ERROR: -10e400 is out of range for type double precision
SELECT '10e-400'::float8;
! float8
!
! 0
! (1 row)
!
SELECT '-10e-400'::float8;
! float8
!
! -0
! (1 row)
!
-- bad input
INSERT INTO FLOAT8_TBL(f1) VALUES ('');
ERROR: invalid input syntax for type double precision:
--- 13,21
SELECT '-10e400'::float8;
ERROR: -10e400 is out of range for type double precision
SELECT '10e-400'::float8;
! ERROR: 10e-400 is out of range for type double precision
SELECT '-10e-400'::float8;
! ERROR: -10e-400 is out of range for type double precision
-- bad input
INSERT INTO FLOAT8_TBL(f1) VALUES ('');
ERROR: invalid input syntax for type double precision:
***
*** 377,383
--- 369,377
INSERT INTO FLOAT8_TBL(f1) VALUES ('-10e400');
ERROR: -10e400 is out of range for type double precision
INSERT INTO FLOAT8_TBL(f1) VALUES ('10e-400');
+ ERROR: 10e-400 is out of range for type double precision
INSERT INTO FLOAT8_TBL(f1) VALUES ('-10e-400');
+ ERROR: -10e-400 is out of range for type double precision
-- maintain external table consistency across platforms
-- delete all values and reinsert well-behaved ones
DELETE FROM FLOAT8_TBL;
=
This happening on cygwin 1.5.20 (running on top of winXP), gcc 3.4.4.
The entire check.log can be found here :
http://www.newsoftcontrol.ro/~am/pgfarm/check.log
The other logs generated by the buildfarm can be found here:
http://www.newsoftcontrol.ro/~am/pgfarm/
Thanks,
Which postgresql version?
Can we have a regular cygwin error report please mailed to cygwin at
cygwin.com please. See http://cygwin.com/problems.html (cygcheck -s -v
-r cygcheck.out)
Looks like a strtod() newlib feature, but I haven't inspected closely.
http://sources.redhat.com/ml/newlib/2006/msg00020.html
BTW: HAVE_LONG_LONG_INT_64 is defined, so INT64_IS_BUSTED is defined also.
Does it look the same on redhat fedora?
Our buildfarm doesn't have these issues,
this runs gcc-3.3.3 and gcc-3.4.4
The problem I see is that float8in() in
src/backend/utils/adt/float.c checks only for -Infinity and not -inf
as returned by newlib:
pg_strncasecmp(num, -Infinity, 9) == 0
Can you test this?
$ cat test-strtod.c
#include stdlib.h
#include stdio.h
#include errno.h
#include float.h
double d;
char *tail;
int main() {
errno = 0;
d = strtod(-10e400, tail);
printf(double: %f, errno: %d, tail: '%s', isinf: %d, fabs: %f,
infmax: %d ,
d, errno, tail, isinf(d), fabs(d), fabs(d) DBL_MAX ? 1 : 0);
}
$ gcc test-strtod.c; ./a
double: -inf, errno: 34, tail: '', isinf: 1, fabs: inf, infmax: 1
--
Reini Urban - postgresql-cygwin maintainer
http://phpwiki.org/ http://murbreak.at/
http://helsinki.at/ http://spacemovie.mur.at/
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] float8 regression failure (HEAD, cygwin)
On 20/07/06, Reini Urban [EMAIL PROTECTED] wrote: Thanks, Which postgresql version? The version is cvs HEAD. Can we have a regular cygwin error report please mailed to cygwin at cygwin.com please. See http://cygwin.com/problems.html (cygcheck -s -v -r cygcheck.out) Looks like a strtod() newlib feature, but I haven't inspected closely. http://sources.redhat.com/ml/newlib/2006/msg00020.html BTW: HAVE_LONG_LONG_INT_64 is defined, so INT64_IS_BUSTED is defined also. Does it look the same on redhat fedora? Our buildfarm doesn't have these issues, this runs gcc-3.3.3 and gcc-3.4.4 I'm not sure about fedora, but on NetBSD 3.0rc5 , postgresql 8.1.3 I can see the same behaviour: am=# select version(); version PostgreSQL 8.1.3 on i386--netbsdelf, compiled by GCC gcc (GCC) 3.3.3 (NetBSD nb3 20040520) (1 row) am=# SELECT '-10e400'::float8; ERROR: -10e400 is out of range for type double precision The problem I see is that float8in() in src/backend/utils/adt/float.c checks only for -Infinity and not -inf as returned by newlib: pg_strncasecmp(num, -Infinity, 9) == 0 Can you test this? Sure . $ gcc test-strtod.c; ./a double: -inf, errno: 34, tail: '', isinf: 1, fabs: inf, infmax: 1 Yes, I'm getting the same output (both on cygwin and netbsd 3.0rc5): $ ./test-strtod.exe double: -inf, errno: 34, tail: '', isinf: 1, fabs: inf,infmax: 1 Cheers, Adrian Maier ---(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: [HACKERS] float8 regression failure (HEAD, cygwin)
Reini Urban [EMAIL PROTECTED] writes: BTW: HAVE_LONG_LONG_INT_64 is defined, so INT64_IS_BUSTED is defined also. You sure? INT64_IS_BUSTED should *not* be set in that case --- it's only supposed to be set if we couldn't find any 64-bit-int type at all. As for the regression test failure, it's odd because it looks to me that the actual test output is an exact match to the default float8.out file. I'm not sure why pg_regress chose to report a diff against float8-small-is-zero.out instead. This may be another teething pain of the new pg_regress-in-C code --- could you trace through it and see what's happening? regards, tom lane ---(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
