Re: [PATCHES] Proposed patch to change TOAST compression strategy

2008-02-29 Thread Teodor Sigaev
Tsvector dump (taken by Magnus from mail archives of pgsql's lists) http://www.sigaev.ru/misc/tstest.sql.bz2 Query: select sum(ts_rank( vector, 'asdfjkl' )) from tstest ; ts_rank detoasts value in any case, even tsvector doesn't contain needed lexemes. Test was on my notebook: Core2 Duo

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor
Tom Lane wrote: I agree with Peter. There are a whole lot of include files that are needed by way more than 3 .c files, and yet are not folded into postgres.h. c.h is right out. My concern is that when we start adding more probes (not just the backend), we will have to add the following 5

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Alvaro Herrera
Robert Lor wrote: My concern is that when we start adding more probes (not just the backend), we will have to add the following 5 lines in .c files that use the Dtrace macros. This seems intrusive and messy to me instead of in a centralized place like c.h. What are the disadvantages for

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor
Alvaro Herrera wrote: Robert Lor wrote: My concern is that when we start adding more probes (not just the backend), we will have to add the following 5 lines in .c files that use the Dtrace macros. This seems intrusive and messy to me instead of in a centralized place like c.h. What

[PATCHES] remove TCL_ARRAYS

2008-02-29 Thread Alvaro Herrera
This patch removes the TCL_ARRAY symbol. This seems to be a leftover from when pgtcl was around in the backend; if enabled, it causes array_out to emit bogus array values: alvherre=# create table bar ( a text); CREATE TABLE alvherre=# insert into bar values ('foo'); INSERT 0 1 alvherre=# select

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor
Peter Eisentraut wrote: Another thing that is concerning me about this new approach is the way the probes are named. For example, we'd now have a call POSTGRESQL_LWLOCK_ACQUIRE() in the code. This does not say we are *tracing* lock aquisition, but it looks like a macro that actually

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Alvaro Herrera
Robert Lor wrote: Alvaro Herrera wrote: Why can't this block be centralized in probes.h? probes.h is auto generated and it can certainly be massaged to include the above logic, but I'd like to avoid doing that if possible. Hmm, so let's have a third file that's not autogenerated, which

Re: [PATCHES] Fix for initdb failures on Vista

2008-02-29 Thread Dave Page
On Fri, Feb 29, 2008 at 3:25 PM, Magnus Hagander [EMAIL PROTECTED] wrote: I noticed that as well when looking at the code, but since I ran my tests on non-vista platforms I didn't hit the actual problem. Dave - it shuold be a simple case of adding the same line of code to the regression

Re: [PATCHES] Fix for initdb failures on Vista

2008-02-29 Thread Magnus Hagander
On Fri, Feb 29, 2008 at 12:17:51AM -0500, Andrew Dunstan wrote: Dave Page wrote: The attached patch fixes problems reported primarily on Vista, but also on some Windows 2003 and XP installations in which initdb reports that it cannot find postgres.exe. This occurs because of

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Tom Lane
Robert Lor [EMAIL PROTECTED] writes: Peter Eisentraut wrote: I understand that these probe names follow some global naming scheme. Is it hard to change that? I'd feel more comfortable with, say, (D)TRACE_POSTGRESQL_LWLOCK_ACQUIRE(). Because the macro is auto generated and follows

Re: [PATCHES] remove TCL_ARRAYS

2008-02-29 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes: This patch removes the TCL_ARRAY symbol. If you're going to remove it you should actually remove it (eg from pg_config_manual.h). This seems to be a leftover from when pgtcl was around in the backend; Yeah, it was supporting some kluge or other in the

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes: Robert Lor wrote: probes.h is auto generated and it can certainly be massaged to include the above logic, but I'd like to avoid doing that if possible. Hmm, so let's have a third file that's not autogenerated, which is the file we will use for

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: Hmm, so let's have a third file that's not autogenerated, which is the file we will use for #includes, and contains just that block. Or just two files. Call probes_null something else, have it be included where needed, have it

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Tom Lane
Robert Lor [EMAIL PROTECTED] writes: I haven't heard any major disadvantages about keeping it in c.h, but if you are still adamant about keeping it out of c.h, I'll will go along with that. I was thinking that pg_trace.h involved some backend-only code, but I'm not sure why I thought that

Re: [PATCHES] CopyReadLineText optimization

2008-02-29 Thread Heikki Linnakangas
Heikki Linnakangas wrote: Attached is a patch that modifies CopyReadLineText so that it uses memchr to speed up the scan. The nice thing about memchr is that we can take advantage of any clever optimizations that might be in libc or compiler. Here's an updated version of the patch. The

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Peter Eisentraut
Am Freitag, 29. Februar 2008 schrieb Robert Lor: My concern is that when we start adding more probes (not just the backend), we will have to add the following 5 lines in .c files that use the Dtrace macros. I had already solved this in my intermediate patch I sent you by symlinking

Re: [PATCHES] Fix for initdb failures on Vista

2008-02-29 Thread Magnus Hagander
Dave Page wrote: On Fri, Feb 29, 2008 at 3:25 PM, Magnus Hagander [EMAIL PROTECTED] wrote: I noticed that as well when looking at the code, but since I ran my tests on non-vista platforms I didn't hit the actual problem. Dave - it shuold be a simple case of adding the same line of code to the

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Peter Eisentraut
Am Freitag, 29. Februar 2008 schrieb Robert Lor: Currently, pg_trace.h is included in c.h, and I feel strongly that it should remains there because by design I'd like to 1) have the tracing feature be available both in the frontend and backend without having to do anything extra, I think

[PATCHES] sinval.c / sinvaladt.c restructuring

2008-02-29 Thread Alvaro Herrera
I just modified the interactions in sinval.c and sinvaladt.c per http://thread.gmane.org/gmane.comp.db.postgresql.devel.patches/18820/focus=18822 It looks a lot saner this way: the code that actually deals with the queue, including locking etc, is all in sinvaladt.c. This means that the struct

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor
Peter Eisentraut wrote: I had already solved this in my intermediate patch I sent you by symlinking probes_null.h to probes.h. Now I see why you created the symlink. But I thinkt the suggestion by Tom/Avaro to include probes.h and the content of probes_null.h in a separate header

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Tom Lane
Robert Lor [EMAIL PROTECTED] writes: And don't think adding a simple comment before the macro call is sufficient? This can be documented so everyone knows the convention. It's stupid. The need for a comment is proof that the macro is badly named. I don't intend to hold still for letting

[PATCHES] Remove FATAL from pg_lzdecompress

2008-02-29 Thread Zdenek Kotala
I attach patch which adds boundaries check and memory overwriting protection when compressed data are corrupted. Current behavior let code overwrite a memory and after that check if unpacked size is same as expected value. In this case elog execution fails (at least on Solaris - malloc has

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor
Tom Lane wrote: We still have what I consider a big problem with the names of the macros. Perhaps that could be fixed by passing the auto-generated file through a sed script to put a prefix on the macro names before we start to use it? Post processing the auto generated header may work,

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Alvaro Herrera
Robert Lor wrote: Post processing the auto generated header may work, but I think it could be unnecessarily complicated and error proned. Would it work to name the traces trace_transaction__start etc instead? AFAICS that would cause the macros to be named

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor
Peter Eisentraut wrote: I think each component would have its own probes definition file. A while back when we met in Toronto, the consensus was to only have one provider called postgresql and all probes whether they be from the backend or frontend will be grouped together in this one

Re: [PATCHES] remove TCL_ARRAYS

2008-02-29 Thread Alvaro Herrera
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: This patch removes the TCL_ARRAY symbol. If you're going to remove it you should actually remove it (eg from pg_config_manual.h). Oops. Thanks, removed from there too. -- Alvaro Herrera

[PATCHES] CopyReadAttributesCSV optimization

2008-02-29 Thread Heikki Linnakangas
Here's a patch to speed up CopyReadAttributesCSV. On the test case I've been playing with, loading the TPC-H partsupp table, about 20% CopyReadAttributesCSV (inlined into DoCopy, DoCopy itself is insignificant): samples %image name symbol name 8136 25.8360 postgres

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor
Alvaro Herrera wrote: Would it work to name the traces trace_transaction__start etc instead? AFAICS that would cause the macros to be named POSTGRESQL_TRACE_TRANSACTION_START() Correct, and that would work. With this approach, all the probe names will start with trace-, and this particular

[PATCHES] Proposing correction to posix_fadvise() usage in xlog.c

2008-02-29 Thread Mark Wong
I believe I have a correction to the usage of posix_fadvise() in xlog.c. Basically posix_fadvise() is being called right before the WAL segment file is closed, which effectively doesn't do anything as opposed to when the file is opened. This proposed correction calls posix_fadvise() in three