Re: [PATCHES] remove BufferBlockPointers for speed and space

2005-08-11 Thread Mark Kirkwood
Mark Kirkwood wrote: Looks to me like -O2 makes the difference very small (on this platform/gcc combo) - is 5/169 worth doing? Ahem - misunderstood your comment here, sorry. Qingqing Zhou wrote: compiled with gcc testbuf.c. I tried -O2 actually, and it turns out that the timing is

Re: [PATCHES] 5 new entries for FAQ

2005-08-11 Thread Martijn van Oosterhout
On Wed, Aug 10, 2005 at 10:04:22AM +0200, Martijn van Oosterhout wrote: After going through pgsql-general a bit I figured there were a few important questions missing from the FAQ, so I wrote some. Given the additions to the FAQ and suggestions made by Bruce about how often the questions come

Re: [PATCHES] remove BufferBlockPointers for speed and space

2005-08-11 Thread Andrew Dunstan
Gavin Sherry wrote: Or more than one hardware architecture (which you didn't even say what you tested...) Well, he tested on SunOS (!) and Linux -- I presume that's two architectures. Sun still calls Solaris SunOs - try doing uname -s on a Solaris box (or look at a buildfarm

Re: [PATCHES] remove BufferBlockPointers for speed and space

2005-08-11 Thread Gavin Sherry
On Thu, 11 Aug 2005, Andrew Dunstan wrote: Gavin Sherry wrote: Or more than one hardware architecture (which you didn't even say what you tested...) Well, he tested on SunOS (!) and Linux -- I presume that's two architectures. Sun still calls Solaris SunOs - try doing uname -s

Re: [PATCHES] PL/pgSQL: #option select_into_1_row (was SELECT INTO

2005-08-11 Thread Matt Miller
On Tue, 2005-08-09 at 15:01 +, Matt Miller wrote: Attached is a patch that implements the #option select_into_1_row directive as suggested. Is this patch good-to-go? Can it be queued? ---(end of broadcast)--- TIP 4: Have you searched our

Re: [PATCHES] PL/pgSQL: #option select_into_1_row (was SELECT INTO

2005-08-11 Thread Bruce Momjian
Matt Miller wrote: On Tue, 2005-08-09 at 15:01 +, Matt Miller wrote: Attached is a patch that implements the #option select_into_1_row directive as suggested. Is this patch good-to-go? Can it be queued? We are in feature freeze, so unless there is overwhelming community support, it

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Andreas Pflug wrote: Bruce Momjian wrote: Dave Page wrote: The only part I didn't like about the patch is the stat display: test= select pg_file_stat('postgresql.conf'); pg_file_stat

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Andreas Pflug
Bruce Momjian wrote: Dave Page wrote: The only part I didn't like about the patch is the stat display: test= select pg_file_stat('postgresql.conf'); pg_file_stat

Re: [PATCHES] remove BufferBlockPointers for speed and space

2005-08-11 Thread Joshua D. Drake
Hello, With gcc-4 on Ubuntu AMD Athlon 2600 Barton: [EMAIL PROTECTED]:~$ ./a.out duration round 1 of array method: 0.539 ms duration round 2 of array method: 0.529 ms duration round 3 of array method: 0.530 ms duration round 1 of mul method: 0.258 ms duration round 2 of mul method: 5.563 ms

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Alvaro Herrera
On Thu, Aug 11, 2005 at 11:34:43AM -0400, Bruce Momjian wrote: Andreas Pflug wrote: Bruce Momjian wrote: Dave Page wrote: The only part I didn't like about the patch is the stat display: test= select pg_file_stat('postgresql.conf');

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Alvaro Herrera
On Thu, Aug 11, 2005 at 01:19:29PM -0400, Bruce Momjian wrote: Alvaro Herrera wrote: So, did you try select * from pg_file_stats('postgresql.conf'); Yes, I got: ERROR: a column definition list is required for functions returning record Interesting. I wonder why the function

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Alvaro Herrera wrote: On Thu, Aug 11, 2005 at 01:19:29PM -0400, Bruce Momjian wrote: Alvaro Herrera wrote: So, did you try select * from pg_file_stats('postgresql.conf'); Yes, I got: ERROR: a column definition list is required for functions returning record

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Alvaro Herrera wrote: On Thu, Aug 11, 2005 at 11:34:43AM -0400, Bruce Momjian wrote: Andreas Pflug wrote: Bruce Momjian wrote: Dave Page wrote: The only part I didn't like about the patch is the stat display: test= select

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Alvaro Herrera
On Thu, Aug 11, 2005 at 02:26:16PM -0400, Bruce Momjian wrote: Alvaro Herrera wrote: Interesting. I wonder why the function is not defined instead with OUT parameters. That way you don't have to declare the record at execution time. See my patch to pgstattuple for an example. Uh,

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
pgman wrote: Andreas Pflug wrote: Bruce Momjian wrote: Dave Page wrote: The only part I didn't like about the patch is the stat display: test= select pg_file_stat('postgresql.conf'); pg_file_stat

Re: [HACKERS] [PATCHES] O_DIRECT for WAL writes

2005-08-11 Thread Mark Wong
Ok, I finally got a couple of tests done against CVS from Aug 3, 2005. I'm not sure if I'm showing anything insightful though. I've learned that fdatasync and O_DSYNC are simply fsync and O_SYNC respectively on Linux, which you guys may have already known. There appears to be a fair performance

Re: [HACKERS] [PATCHES] O_DIRECT for WAL writes

2005-08-11 Thread Bruce Momjian
Mark Wong wrote: Ok, I finally got a couple of tests done against CVS from Aug 3, 2005. I'm not sure if I'm showing anything insightful though. I've learned that fdatasync and O_DSYNC are simply fsync and O_SYNC respectively on Linux, which you guys may have already known. There appears to

Re: [PATCHES] [HACKERS] Autovacuum loose ends

2005-08-11 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes: Updated this patch again: - vacuum_cost_delay and vacuum_cost_limit can be set per table, as well as globally with autovacuum_vacuum_cost_{limit,delay} - pgstat is reset if recovery is required - pgstat reset at postmaster start is disabled by

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes: Interesting. I wonder why the function is not defined instead with OUT parameters. Because bootstrap mode isn't capable of dealing with array columns, so you can't define stuff in pg_proc.h that sets up an array of OUT parameter types. I tried to apply

Re: [PATCHES] [HACKERS] Autovacuum loose ends

2005-08-11 Thread Alvaro Herrera
On Thu, Aug 11, 2005 at 05:13:15PM -0400, Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: Updated this patch again: Applied with minor tweaks --- mostly, fixing it so the custom cost settings are applied for ANALYZE as well as VACUUM. Ok, cool, thanks. I think this completes the

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: Interesting. I wonder why the function is not defined instead with OUT parameters. Because bootstrap mode isn't capable of dealing with array columns, so you can't define stuff in pg_proc.h that sets up an array of OUT parameter

Re: [PATCHES] BUG #1467: fe_connect doesn't handle EINTR right

2005-08-11 Thread Tom Lane
AgentM [EMAIL PROTECTED] writes: Attached is a patch which corrects the behavior. I verified that the patch does not interfere with normal operation (using psql) but unfortunately the code path is virtually impossible to test without a really slow connection to a postgresql server [which

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread William ZHANG
What if we let the trailing . or .. as it is? On Windows, GetFullPathName() can canonicalize a given path. $ uname -a MINGW32_NT-5.0 DEV 1.0.10(0.46/3/2) 2003-10-11 10:14 i686 unknown $ cat path.c #include windows.h int main(void) { CHAR Buf[1024]; GetFullPathName(C:\\Temp,

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Bruce Momjian
Yep, it is a bug on 8.0.X. Are you suggesting a backpatch? We can do it. Any objections? --- William ZHANG wrote: What if we let the trailing . or .. as it is? On Windows, GetFullPathName() can canonicalize a given

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes: Yep, it is a bug on 8.0.X. Are you suggesting a backpatch? We can do it. Any objections? I think he's suggesting that we depend on GetFullPathName(), which seems a bit pointless considering we have to write the code anyway for other platforms. I

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Bruce Momjian
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: Yep, it is a bug on 8.0.X. Are you suggesting a backpatch? We can do it. Any objections? I think he's suggesting that we depend on GetFullPathName(), which seems a bit pointless considering we have to write the code anyway

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread uniware
- Original Message - From: Tom Lane [EMAIL PROTECTED] To: Bruce Momjian pgman@candle.pha.pa.us Cc: William ZHANG [EMAIL PROTECTED]; pgsql-patches@postgresql.org Sent: Friday, August 12, 2005 10:31 AM Subject: Re: [PATCHES] Bug in canonicalize_path() Bruce Momjian

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Tom Lane
I wrote: I didn't much like the patch as-is; we should think a little harder about fixing the logic properly. Like, say, this. (Very poorly tested but I think it's right.) regards, tom lane *** src/port/path.c.origThu Aug 11 21:39:22 2005 --- src/port/path.c

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes: And then you have this case: /usr/local/../bin/../.. AFAICS, the patch I just proposed handles this. If I recall the code properly, we do not have to make canonicalize_path remove embedded . or .. --- that is, we do not have to simplify

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Bruce Momjian
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: And then you have this case: /usr/local/../bin/../.. AFAICS, the patch I just proposed handles this. If I recall the code properly, we do not have to make canonicalize_path remove embedded . or .. --- that is, we do not

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes: But what about usr/local/../../..? What about it? The case of /usr/local/../../.. is handled correctly, and the case where it's an underspecified relative path doesn't seem that interesting to me --- certainly that is not so important that we should

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Bruce Momjian
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: But what about usr/local/../../..? What about it? The case of /usr/local/../../.. is handled correctly, and the case where it's an underspecified relative path doesn't seem that interesting to me --- certainly that is not so

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread William ZHANG
I have test the following on Windows and Linux: Windows: C:\cd Winnt C:\WINNTcd C:\Temp\..\..\ C:\ Linux: $ cd /usr/../../ $ pwd / We should handle this correctly. 1 Single dot in the path can be removed safety. (except the first one. e.g. ./usr/local) 2 Every

Re: [PATCHES] remove BufferBlockPointers for speed and space

2005-08-11 Thread Qingqing Zhou
Tom Lane [EMAIL PROTECTED] writes: This is definitely going to tell us a lot more about the compiler's loop strength reduction algorithms than it is going to tell about the time to evaluate any one of these expressions in isolation. What's worse, the compiler would be quite within its

Re: [PATCHES] [HACKERS] For review: Server instrumentation patch

2005-08-11 Thread Bruce Momjian
Here is an updated patch I have just applied (catalog version updated). I have added these functions: pg_stat_file() pg_read_file() pg_ls_dir() pg_reload_conf() pg_rotate_logfile() I did not include pg_logdir_ls because that was basically pg_ls_dir with

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes: I figured it would be best to leave it alone if we can't process it, but if you think it is more imporant to trim in cases like ../.., go ahead. My recollection of this patch: 2004-08-09 16:20 tgl * src/: port/exec.c, port/path.c,

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Bruce Momjian
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: I figured it would be best to leave it alone if we can't process it, but if you think it is more imporant to trim in cases like ../.., go ahead. My recollection of this patch: 2004-08-09 16:20 tgl * src/:

Re: [PATCHES] remove BufferBlockPointers for speed and space

2005-08-11 Thread Tom Lane
Qingqing Zhou [EMAIL PROTECTED] writes: I've patched the code according to your suggestion. Result is: [ snip ] OK, that test seems a little more believable. One point you didn't consider is that on 64-bit machines, the integer bufnum really has to be coerced to size_t to avoid overflow if the

Re: [PATCHES] Bug in canonicalize_path()

2005-08-11 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes: Tom Lane wrote: ... it's part of the API contract of canonicalize_path() that it will not return something with trailing . or ... OK, new patch which I think handles all cases. + if (pending_strips 0) + { + for (;