On Sun, 11 May 2008, Hans-Juergen Schoenig wrote:

we also made some simple autoconf hack to check for broken posix_fadvise.


Because of how you did that, your patch is extremely difficult to even test. You really should at least scan the output from diff you're about to send before submitting a patch to make sure it makes sense--yours is over 30,000 lines long just to implement a small improvement. The reason for that is that you regenerated "configure" using a later version of Autoconf than the official distribution, and the diff for the result is gigantic. You even turned off the check in configure.in that specifically prevents you from making that mistake so it's not like you weren't warned.

You should just show the necessary modifications to configure.in in your patch, certainly shouldn't submit a patch that subverts the checks there, and leave out the resulting configure file if you didn't use the same version of Autoconf.

I find the concept behind this patch very useful and I'd like to see a useful one re-submitted. I'm in the middle of setting up some new hardware this month and was planning to test the existing fadvise patches Greg Stark sent out as part of that. Having another one to test for accelerating multiple sequential scans would be extremely helpful to add to that, because then I think I can reuse some of the test cases Jeff Davis put together for the 8.3 improvements in that area to see how well it works. It wasn't as clear to me how to test the existing bitmap scan patch, yours seems a more straightforward patch to use as a testing ground for fadvise effectiveness.

--
* Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Reply via email to