Re: [HACKERS] Feedback on writing extensible modules

2009-07-06 Thread Dimitri Fontaine 
Tom Lane t...@sss.pgh.pa.us writes:

 Dimitri Fontaine dfonta...@hi-media.com writes:
 Please find attached a little little patch which run  
 process_local_preload_libraries from within a transaction.

 This is inevitably going to break other people's code.  Put the
 transaction wrapper in your own stuff if you have to have it.

The module is working fine on HEAD without any patch if it cares about
starting a transaction itself into _PG_init(), even when _PG_init() is
called at function call time rather than at local_preload_libraries
time.

My reserve was that I thought the transaction arround _PG_init() was
existing in a 'normal' call, so the explicit creation of it in the
module would fail:
StartTransactionCommand();
...
CommitTransactionCommand();

Now my only problem is related to making the module 8.3 compliant:

pre_prepare.c:19:27: error: utils/snapmgr.h: No such file or directory
pre_prepare.c: In function ‘_PG_init’:
pre_prepare.c:188: warning: implicit declaration of function 
‘PushActiveSnapshot’
pre_prepare.c:207: warning: implicit declaration of function ‘PopActiveSnapshot’

I guess I can document that having pre_prepare in
local_preload_libraries with preprepare.at_init = on is only support
from 8.4 onward...

Regards,
-- 
dim

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

Re: [HACKERS] Feedback on writing extensible modules

2009-07-05 Thread Dimitri Fontaine 

Hi,

Le 31 mai 09 à 18:21, Tom Lane a écrit :

The reason this doesn't work is that SPI can only be invoked inside a
transaction, and you're not inside one when a library is being
preloaded.


Please find attached a little little patch which run  
process_local_preload_libraries from within a transaction.


The following patch to preprepare makes it working fine when the GUC  
preprepare.at_init is on and the module is being loaded from  
local_preload_librairies:

  
http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/preprepare/preprepare/pre_prepare.c.diff?r1=1.1r2=1.2


You could maybe make this work by executing your own transaction
to do it


I took the option to have PostgreSQL provide the same context in  
preloading that when loading later, in that in both case _PG_init()  
runs inside an already existing transaction. I don't see a current way  
for _PG_init() to distinguish between being called at backend fork()  
time or from a user transaction, so figured it was better this way.



but I really have to wonder if it's a good idea.  One
point to think about is that elog(ERROR) still means elog(FATAL)
at this point, so any brokenness in the queries you're trying to
prepare will result in locking all users out of the database.



Well loading custom code at init time is a foot-gun reserved to  
superusers with access to the local file system (where to put the  
custom .so) and allowed to signal postmaster. You're right that a  
failure in the module init routine will prevent anyone from connecting  
to the server, but the cure is to clean local_preload_librairies then  
restart.
Or with the preprepare example, to set preprepare.at_init to off then  
reload.


Regards,
--
dim



preload-spi.diff
Description: Binary data




PS: sorry I don't have the toolsuite to provide a context diff  
tonight, but given the size of the patch I figured I'd send it anyway.  
Will cook a context diff tomorrow if needed, it's running late here.  
Oh, we're already tomorrow, even.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Feedback on writing extensible modules

2009-07-05 Thread Tom Lane 
Dimitri Fontaine dfonta...@hi-media.com writes:
 Please find attached a little little patch which run  
 process_local_preload_libraries from within a transaction.

This is inevitably going to break other people's code.  Put the
transaction wrapper in your own stuff if you have to have it.

regards, tom lane

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

Re: [HACKERS] Feedback on writing extensible modules

2009-06-01 Thread Alvaro Herrera 
Dimitri Fontaine wrote:

 Le 31 mai 09 à 18:21, Tom Lane a écrit :

 You could maybe make this work by executing your own transaction
 to do it, but I really have to wonder if it's a good idea.  One
 point to think about is that elog(ERROR) still means elog(FATAL)
 at this point, so any brokenness in the queries you're trying to
 prepare will result in locking all users out of the database.

 Yeah that's a pretty good foot gun, yet another one. But  
 preprepare.at_init is optional and defaults to off. If you broke it all, 
 you can turn it off again and reload.

Maybe you could set a callback to be called during the first transaction
in InitPostgres ... in full knowledge that if it fails, people will be
locked out of the database.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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

Re: [HACKERS] Feedback on writing extensible modules

2009-06-01 Thread Simon Riggs 

On Mon, 2009-06-01 at 12:23 -0400, Alvaro Herrera wrote:
 Dimitri Fontaine wrote:
 
  Le 31 mai 09 à 18:21, Tom Lane a écrit :
 
  You could maybe make this work by executing your own transaction
  to do it, but I really have to wonder if it's a good idea.  One
  point to think about is that elog(ERROR) still means elog(FATAL)
  at this point, so any brokenness in the queries you're trying to
  prepare will result in locking all users out of the database.
 
  Yeah that's a pretty good foot gun, yet another one. But  
  preprepare.at_init is optional and defaults to off. If you broke it all, 
  you can turn it off again and reload.
 
 Maybe you could set a callback to be called during the first transaction
 in InitPostgres ... in full knowledge that if it fails, people will be
 locked out of the database.

Should be possible to define a custom variable that has an assign hook
that does nothing unless called with PGC_S_DATABASE or PGC_S_USER. That
should guarantee the code only runs after connection, rather than at
server startup. Not tried it yet.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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

Re: [HACKERS] Feedback on writing extensible modules

2009-05-31 Thread Tom Lane 
Dimitri Fontaine dfonta...@hi-media.com writes:
 Dimitri Fontaine dfonta...@hi-media.com writes:
 And currently calling SPI_connect() from _PG_init will crash the
 backend. I'll try to obtain a gdb backtrace, I've just been told about
 pre_auth_delay and post_auth_delay parameters.

 Here we go:

The reason this doesn't work is that SPI can only be invoked inside a
transaction, and you're not inside one when a library is being
preloaded.

 I'm very interrested in being able
 to prepare a query at local_preload_libraries time, if possible in 8.3
 and following releases.

You could maybe make this work by executing your own transaction
to do it, but I really have to wonder if it's a good idea.  One
point to think about is that elog(ERROR) still means elog(FATAL)
at this point, so any brokenness in the queries you're trying to
prepare will result in locking all users out of the database.

regards, tom lane

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

Re: [HACKERS] Feedback on writing extensible modules

2009-05-31 Thread Dimitri Fontaine 

Hi,

First, thank you to have taken the time to see about the case.

Le 31 mai 09 à 18:21, Tom Lane a écrit :

The reason this doesn't work is that SPI can only be invoked inside a
transaction, and you're not inside one when a library is being
preloaded.


Makes sense. Still crashing with basic naive testing, will report back  
when I have more time to work on it.



You could maybe make this work by executing your own transaction
to do it, but I really have to wonder if it's a good idea.  One
point to think about is that elog(ERROR) still means elog(FATAL)
at this point, so any brokenness in the queries you're trying to
prepare will result in locking all users out of the database.


Yeah that's a pretty good foot gun, yet another one. But  
preprepare.at_init is optional and defaults to off. If you broke it  
all, you can turn it off again and reload.


As for the FATAL, I guess that having preprepare crashing backend  
creations rather than having your EXECUTE fail and ROLLBACK your  
transactions is not so much a difference when you need preprepare in  
the first place...
I'll add a note in the documentation to always manually call SELECT  
prepare_all() at each prepare statements list modification before to  
turn at_init on, as soon as at_init is possible.


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

Re: [HACKERS] Feedback on writing extensible modules

2009-05-25 Thread Dimitri Fontaine 
Dimitri Fontaine dfonta...@hi-media.com writes:
 And currently calling SPI_connect() from _PG_init will crash the
 backend. I'll try to obtain a gdb backtrace, I've just been told about
 pre_auth_delay and post_auth_delay parameters.

Here we go:

(gdb) handle SIGABRT nopass
SignalStop  Print   Pass to program Description
SIGABRT   Yes   Yes No  Aborted
(gdb) continue
Program received signal SIGABRT, Aborted.
0xb802d424 in __kernel_vsyscall ()
(gdb) bt
#0  0xb802d424 in __kernel_vsyscall ()
#1  0xb7e7c640 in raise () from /lib/i686/cmov/libc.so.6
#2  0xb7e7dfa1 in abort () from /lib/i686/cmov/libc.so.6
#3  0x082dadde in ExceptionalCondition (conditionName=0x83cbfe0 !(((context) 
!= ((void *)0)  (Node*)((context)))-type) == T_AllocSetContext, 
errorType=0x830bc09 BadArgument, fileName=0x83be166 mcxt.c, 
lineNumber=507) at assert.c:57
#4  0x082f8abb in MemoryContextAlloc (context=0x0, size=448) at mcxt.c:507
#5  0x081a93a3 in SPI_connect () at spi.c:81
#6  0xb582cf15 in _PG_init () at pre_prepare.c:150
#7  0x082df913 in internal_load_library (libname=0x9808da4 
/home/dim/pgsql/8.3/lib/plugins/pre_prepare.so) at dfmgr.c:296
#8  0x082dfc38 in load_file (filename=0x9809d00 $libdir/plugins/pre_prepare, 
restricted=1 '\001') at dfmgr.c:153
#9  0x082e7554 in load_libraries (libraries=value optimized out, 
gucname=0x9809d00 $libdir/plugins/pre_prepare, restricted=1 '\001') at 
miscinit.c:1185
#10 0x08233ce2 in PostgresMain (argc=4, argv=0x9807fb8, username=0x9807f90 
dim) at postgres.c:3314
#11 0x0820054c in ServerLoop () at postmaster.c:3207
#12 0x0820124b in PostmasterMain (argc=3, argv=0x97f1bd8) at postmaster.c:1029
#13 0x081b2b39 in main (argc=3, argv=0x97f1bd8) at main.c:188

And I'm runnin a CVS version of 8.3 I'm not sure is the last update in
the branch, so here's what I have at mcxt.c:507

504 void *
505 MemoryContextAlloc(MemoryContext context, Size size)
506 {
507 AssertArg(MemoryContextIsValid(context));
508
509 if (!AllocSizeIsValid(size))
510 elog(ERROR, invalid memory alloc request size %lu,
511  (unsigned long) size);

That's with attached patch to pre_prepare.c from pgfoundry:
  http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/preprepare/preprepare/

If you need any more information from me, or for me to rerun with
another server version, please ask. I'm very interrested in being able
to prepare a query at local_preload_libraries time, if possible in 8.3
and following releases.

Regards,
-- 
dim

Index: pre_prepare.c
===
RCS file: /cvsroot/preprepare/preprepare/pre_prepare.c,v
retrieving revision 1.1
diff -p -u -r1.1 pre_prepare.c
--- pre_prepare.c	13 May 2009 20:54:04 -	1.1
+++ pre_prepare.c	25 May 2009 13:37:52 -
@@ -35,6 +35,7 @@
 
 PG_MODULE_MAGIC;
 
+static bool  pre_prepare_at_init  = NULL;
 static char *pre_prepare_relation = NULL;
 
 void _PG_init(void);
@@ -125,6 +126,15 @@ int pre_prepare_all() {
  */
 void
 _PG_init(void) {
+  DefineCustomBoolVariable(preprepare.at_init,
+			   Do we prepare the statements at backend init start,
+			   You have to setup local_preload_libraries too,
+			   pre_prepare_at_init,
+			   PGC_USERSET,
+			   NULL, 
+			   NULL);
+  EmitWarningsOnPlaceholders(prepare.at_init);
+
   DefineCustomStringVariable(preprepare.relation,
 			 Table name where to find statements to prepare,
 			 Can be schema qualified, must have columns \name\ and \statement\,
@@ -132,8 +142,21 @@ _PG_init(void) {
 			 PGC_USERSET,
 			 NULL, 
 			 NULL);
-
   EmitWarningsOnPlaceholders(prepare.relation);
+
+  if( pre_prepare_at_init ) {
+int err;
+
+err = SPI_connect();
+if (err != SPI_OK_CONNECT)
+  elog(ERROR, SPI_connect: %s, SPI_result_code_string(err));
+
+pre_prepare_all();
+
+err = SPI_finish();
+if (err != SPI_OK_FINISH)
+  elog(ERROR, SPI_finish: %s, SPI_result_code_string(err));
+  }
 }
 
 /*

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

[HACKERS] Feedback on writing extensible modules

2009-05-20 Thread Simon Riggs 

Some feedback

1. Want some very clear and supported way to know whether Postgres is
fully up. Currently, if you write _PG_init you sometimes need to know if
it is being executed by LOAD or as a reload. So actual initialisation
sometimes needs to happen outside of _PG_init.

2. shmem_startup_hook doesn't allow multiple modules to create shmem.
All callers of the hook think they are the only caller, causing chaos if
multiple people need this. Currently, whoever sets up the hook gets to
create shmem. (There's no docs for this yet). Would prefer something
like RequestAddinShmemSpace() which can be called by multiple callers.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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

Re: [HACKERS] Feedback on writing extensible modules

2009-05-20 Thread Dimitri Fontaine 

Hi,

Le 20 mai 09 à 20:51, Simon Riggs a écrit :

1. Want some very clear and supported way to know whether Postgres is
fully up. Currently, if you write _PG_init you sometimes need to  
know if

it is being executed by LOAD or as a reload. So actual initialisation
sometimes needs to happen outside of _PG_init.


And currently calling SPI_connect() from _PG_init will crash the  
backend. I'll try to obtain a gdb backtrace, I've just been told about  
pre_auth_delay and post_auth_delay parameters.


Regards,
--
dim


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

Re: [HACKERS] Feedback on writing extensible modules

2009-05-20 Thread Tom Lane 
Simon Riggs si...@2ndquadrant.com writes:
 2. shmem_startup_hook doesn't allow multiple modules to create shmem.
 All callers of the hook think they are the only caller, causing chaos if
 multiple people need this.

The only known caller, contrib/pg_stat_statements/, does not think that.
Do what it does.

I agree it's undocumented, but so are the other hooks ...

regards, tom lane

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

Re: [HACKERS] Feedback on writing extensible modules

2009-05-20 Thread Simon Riggs 

On Wed, 2009-05-20 at 16:03 -0400, Tom Lane wrote:
 Simon Riggs si...@2ndquadrant.com writes:
  2. shmem_startup_hook doesn't allow multiple modules to create shmem.
  All callers of the hook think they are the only caller, causing chaos if
  multiple people need this.
 
 The only known caller, contrib/pg_stat_statements/, does not think that.
 Do what it does.

Hmmm, it works, I guess. As long as everyone does that. 

Thanks for the workaround.

 I agree it's undocumented, but so are the other hooks ...

Some are well documented in the code, not all though.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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