Re: [PATCHES] [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 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [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 6: explain analyze is your friend
Re: [PATCHES] [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 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] [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: [PATCHES] [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 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] float8 regression failure (HEAD, cygwin)
[ re cassowary buildfarm failure ] "Adrian Maier" <[EMAIL PROTECTED]> writes: > On 20/07/06, Tom Lane <[EMAIL PROTECTED]> wrote: >> 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. > 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 Doh ... the question though is why are you getting different results from everybody else? There are other cygwin machines in the buildfarm and they are all passing regression --- I suppose they'd start failing if we remove that resultmap entry. The regular float8 result is certainly "more correct" than float8-small-is-zero, so I'm all for removing the resultmap entry if we can do it. But we'd need to be able to explain to people how to get their machines to pass, and right now I don't know what to say. 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: [PATCHES] [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: [PATCHES] [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 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] 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 4: Have you searched our list archives? http://archives.postgresql.org
