Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-03-02 Thread Marko Tiikkaja

Hi Pavel,

The extra semicolons are still in there; around line 525 in this patch. 
 However, I removed them to compile the patch, but I can't compile my 
plugin on OS X.  The plugin is simple, it just does:


void
_PG_init(void)
{
DirectFunctionCall1(plpgsql_register_plugin, 
pgt_plpgsql_plugin_struct);

}

I get:

Undefined symbols for architecture x86_64:
  _plpgsql_register_plugin, referenced from:
  __PG_init in plpgtest.o

I'm guessing this is because PL/PgSQL is a shared library and not in 
core?  Is there a way around this?



Regards,
Marko Tiikkaja


--
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] proposal, patch: allow multiple plpgsql plugins

2014-03-02 Thread Pavel Stehule
Hi


2014-03-02 19:59 GMT+01:00 Marko Tiikkaja ma...@joh.to:

 Hi Pavel,

 The extra semicolons are still in there; around line 525 in this patch.
  However, I removed them to compile the patch, but I can't compile my
 plugin on OS X.  The plugin is simple, it just does:

 void
 _PG_init(void)
 {
 DirectFunctionCall1(plpgsql_register_plugin,
 pgt_plpgsql_plugin_struct);
 }

 I get:

 Undefined symbols for architecture x86_64:
   _plpgsql_register_plugin, referenced from:
   __PG_init in plpgtest.o

 I'm guessing this is because PL/PgSQL is a shared library and not in core?
  Is there a way around this?


yes, PLpgSQL is not referenced and, if I remember well, clang is too
restrictive.

probably
http://stackoverflow.com/questions/17281901/ignoring-an-undefined-symbol-in-a-dynamic-library-from-xcode

or you can add a reference on plpgsql to your Makefile

Regards

Pavel



 Regards,
 Marko Tiikkaja



Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-03-02 Thread Marko Tiikkaja

On 3/2/14, 8:47 PM, Pavel Stehule wrote:

2014-03-02 19:59 GMT+01:00 Marko Tiikkaja ma...@joh.to:

Undefined symbols for architecture x86_64:
   _plpgsql_register_plugin, referenced from:
   __PG_init in plpgtest.o

I'm guessing this is because PL/PgSQL is a shared library and not in core?
  Is there a way around this?



yes, PLpgSQL is not referenced and, if I remember well, clang is too
restrictive.

probably
http://stackoverflow.com/questions/17281901/ignoring-an-undefined-symbol-in-a-dynamic-library-from-xcode

or you can add a reference on plpgsql to your Makefile


That seems unbelievably ugly, but worse, loading the library in 
shared_preload_libraries doesn't work:


14782  FATAL:  could not load library 
/usr/local/pgsql/lib/plpgtest.so: 
dlopen(/usr/local/pgsql/lib/plpgtest.so, 10): Symbol not found: 
_plpgsql_register_plugin

  Referenced from: /usr/local/pgsql/lib/plpgtest.so
  Expected in: flat namespace
 in /usr/local/pgsql/lib/plpgtest.so

I even tried putting plpgsql.so before it in the list, but no go.


Regards,
Marko Tiikkaja


--
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] proposal, patch: allow multiple plpgsql plugins

2014-03-02 Thread Pavel Stehule
Dne 2. 3. 2014 21:55 Marko Tiikkaja ma...@joh.to napsal(a):

 On 3/2/14, 8:47 PM, Pavel Stehule wrote:

 2014-03-02 19:59 GMT+01:00 Marko Tiikkaja ma...@joh.to:

 Undefined symbols for architecture x86_64:

_plpgsql_register_plugin, referenced from:
__PG_init in plpgtest.o

 I'm guessing this is because PL/PgSQL is a shared library and not in
core?
   Is there a way around this?


 yes, PLpgSQL is not referenced and, if I remember well, clang is too
 restrictive.

 probably

http://stackoverflow.com/questions/17281901/ignoring-an-undefined-symbol-in-a-dynamic-library-from-xcode

 or you can add a reference on plpgsql to your Makefile


 That seems unbelievably ugly, but worse, loading the library in
shared_preload_libraries doesn't work:

 14782  FATAL:  could not load library /usr/local/pgsql/lib/plpgtest.so:
dlopen(/usr/local/pgsql/lib/plpgtest.so, 10): Symbol not found:
_plpgsql_register_plugin
   Referenced from: /usr/local/pgsql/lib/plpgtest.so
   Expected in: flat namespace
  in /usr/local/pgsql/lib/plpgtest.so

 I even tried putting plpgsql.so before it in the list, but no go.



 Regards,
 Marko Tiikkaja
In this moment, pls, try to use Load plpgsql

Regards

pavel


Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-03-02 Thread Pavel Stehule
2014-03-03 6:09 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:


 Dne 2. 3. 2014 21:55 Marko Tiikkaja ma...@joh.to napsal(a):

 
  On 3/2/14, 8:47 PM, Pavel Stehule wrote:
 
  2014-03-02 19:59 GMT+01:00 Marko Tiikkaja ma...@joh.to:
 
  Undefined symbols for architecture x86_64:
 
 _plpgsql_register_plugin, referenced from:
 __PG_init in plpgtest.o
 
  I'm guessing this is because PL/PgSQL is a shared library and not in
 core?
Is there a way around this?
 
 
  yes, PLpgSQL is not referenced and, if I remember well, clang is too
  restrictive.
 
  probably
 
 http://stackoverflow.com/questions/17281901/ignoring-an-undefined-symbol-in-a-dynamic-library-from-xcode
 
  or you can add a reference on plpgsql to your Makefile
 
 
  That seems unbelievably ugly, but worse, loading the library in
 shared_preload_libraries doesn't work:
 
  14782  FATAL:  could not load library
 /usr/local/pgsql/lib/plpgtest.so:
 dlopen(/usr/local/pgsql/lib/plpgtest.so, 10): Symbol not found:
 _plpgsql_register_plugin
Referenced from: /usr/local/pgsql/lib/plpgtest.so
Expected in: flat namespace
   in /usr/local/pgsql/lib/plpgtest.so
 
  I even tried putting plpgsql.so before it in the list, but no go.

 
 
  Regards,
  Marko Tiikkaja
 In this moment, pls, try to use Load plpgsql

I though about it this morning - we should to move plugin registration to
core - it should to work like ddl loader

a) it can solve problems with loading
b) it can be usable for all PL environment.

Pavel



 Regards

 pavel



Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-02-10 Thread Pavel Stehule
Hello Marko


2014-01-16 23:54 GMT+01:00 Marko Tiikkaja ma...@joh.to:

 Hi Pavel,

 First of all, thanks for working on this!


 On 1/12/14, 8:58 PM, Pavel Stehule wrote:

 I still not happy with plugin_info - it is only per plugin now and should
 be per plugin and per function.


 I'm not sure I understand the point of plugin_info in the first place, but
 what would having a separate info per (plugin, function) achieve that can't
 be done otherwise?


 As for the current patch, I'd like to see improvements on a few things:

   1) It doesn't currently compile because of extra semicolons in the
  PLpgSQL_plugin struct.


fixed



   2) The previous comment above the same struct still talk about the
  rendezvous variable which is now gone.  The comment should be
  updated to reflect the new API.


removed



   3) The same comment talks about how important it is to unregister a
  plugin if its _PG_fini() is ever called, but the current API does
  not support unregistering.  That should probably be added?  I'm not
  sure when _PG_fini() would be called.


removed

These plugins should not be removed - there is no any mechanism how to
remove active plugin without close session

Regards

Pavel



   4) The comment  /* reserved for use by optional plugin */  seems a bit
  weird in its new context.


 Regards,
 Marko Tiikkaja

commit bf0820786f08b08812bba3d86233cbac30922054
Author: Pavel Stehule pavel.steh...@gooddata.com
Date:   Mon Feb 10 17:50:31 2014 +0100

initial

diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index 852b0c7..37d17a8 100644
--- a/src/pl/plpgsql/src/Makefile
+++ b/src/pl/plpgsql/src/Makefile
@@ -19,7 +19,7 @@ rpath =
 
 OBJS = pl_gram.o pl_handler.o pl_comp.o pl_exec.o pl_funcs.o pl_scanner.o
 
-DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql
+DATA = plpgsql.control plpgsql--1.1.sql plpgsql--1.0--1.1.sql plpgsql--unpackaged--1.0.sql
 
 all: all-lib
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 3749fac..e6510f1 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -86,6 +86,22 @@ typedef struct SimpleEcontextStackEntry
 static EState *shared_simple_eval_estate = NULL;
 static SimpleEcontextStackEntry *simple_econtext_stack = NULL;
 
+/*
+ * List of pointers and info of registered plugins.
+ */
+typedef struct PluginPtrEntry
+{
+	PLpgSQL_plugin *plugin_ptr;
+	struct PluginPtrEntry *next;
+} PluginPtrEntry;
+
+/*
+ * Allocated in TopMemoryContext
+ */
+static PluginPtrEntry *plugins = NULL;
+static int nplugins = 0;
+static int used_plugin_hook_types = 0;
+
 /
  * Local function forward declarations
  /
@@ -235,6 +251,18 @@ static char *format_expr_params(PLpgSQL_execstate *estate,
 const PLpgSQL_expr *expr);
 static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
 	   const PreparedParamsData *ppd);
+static void **get_plugin_info(PLpgSQL_execstate *estate, int plugin_number);
+
+
+/* Bits for used plugin callback types */
+#define PLUGIN_HOOK_TYPE_FUNC_SETUP	(1  0)
+#define PLUGIN_HOOK_TYPE_FUNC_BEG	(1  2)
+#define PLUGIN_HOOK_TYPE_FUNC_END	(1  3)
+#define PLUGIN_HOOK_TYPE_STMT_BEG	(1  4)
+#define PLUGIN_HOOK_TYPE_STMT_END	(1  5)
+
+#define EXEC_PLUGIN_HOOK_TYPE(type) \
+	((used_plugin_hook_types  (PLUGIN_HOOK_TYPE_ ## type)) == (PLUGIN_HOOK_TYPE_ ## type))
 
 
 /* --
@@ -331,10 +359,28 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	exec_set_found(estate, false);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_beg)
-		((*plugin_ptr)-func_beg) (estate, func);
+	if (EXEC_PLUGIN_HOOK_TYPE(FUNC_BEG))
+	{
+		PluginPtrEntry *pe;
+		int i;
+
+		Assert(plugins != NULL);
+
+		for (i = 0, pe = plugins; pe != NULL; pe = pe-next, i++)
+		{
+			PLpgSQL_plugin *pl_ptr = pe-plugin_ptr;
+
+			if (pl_ptr-func_beg)
+			{
+void **plugin_info;
+
+plugin_info = get_plugin_info(estate, i);
+(pl_ptr-func_beg) (estate, func, plugin_info);
+			}
+		}
+	}
 
 	/*
 	 * Now call the toplevel block of statements
@@ -479,10 +525,28 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	estate.err_text = gettext_noop(during function exit);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_end)
-		((*plugin_ptr)-func_end) (estate, func);
+	if (EXEC_PLUGIN_HOOK_TYPE(FUNC_END))
+	{
+		PluginPtrEntry *pe;
+		int i;
+
+		Assert(plugins != NULL);
+
+		for (i = 0, pe = plugins; pe != NULL; i++, pe = pe-next)
+		{
+			PLpgSQL_plugin *pl_ptr = pe-plugin_ptr;
+
+			if (pl_ptr-func_end)
+			{
+void **plugin_info;
+
+

Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-02-09 Thread Pavel Stehule
Hello

updated patch - now plugin_info is per plpgsq_estate/plugin again.

Regards

Pavel


2014-01-17 20:26 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:




 2014/1/16 Marko Tiikkaja ma...@joh.to

 Hi Pavel,

 First of all, thanks for working on this!


 On 1/12/14, 8:58 PM, Pavel Stehule wrote:

 I still not happy with plugin_info - it is only per plugin now and should
 be per plugin and per function.


 I'm not sure I understand the point of plugin_info in the first place,
 but what would having a separate info per (plugin, function) achieve that
 can't be done otherwise?


 First use case - I would to protect repeated call of
 plpgsql_check_function in passive mode. Where I have to store information
 about successful first start? It is related to the function instance, so
 function oid can be ambiguous (for function with polymorphic parameters).
 When function instance is destroyed, then this information should be
 destroyed. It is impossible do this check from plugin. Second use case -
 attach session life cycle plugin data with some function - for example for
 coverage calculation. Inside plugin without function specific data you have
 to hold a hash of all used function, and you have to search again and
 again. When plpgsql hold this info in internal plpgsql function structures,
 then you don't need search anything.




 Regards

 Pavel





 As for the current patch, I'd like to see improvements on a few things:

   1) It doesn't currently compile because of extra semicolons in the
  PLpgSQL_plugin struct.

   2) The previous comment above the same struct still talk about the
  rendezvous variable which is now gone.  The comment should be
  updated to reflect the new API.

   3) The same comment talks about how important it is to unregister a
  plugin if its _PG_fini() is ever called, but the current API does
  not support unregistering.  That should probably be added?  I'm not
  sure when _PG_fini() would be called.

   4) The comment  /* reserved for use by optional plugin */  seems a bit
  weird in its new context.


 Regards,
 Marko Tiikkaja



commit eecd714579b3683b02a814b685b1d559ba0e0da8
Author: Pavel Stehule pavel.steh...@gmail.com
Date:   Sun Feb 9 22:08:18 2014 +0100

initial

diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index 852b0c7..37d17a8 100644
--- a/src/pl/plpgsql/src/Makefile
+++ b/src/pl/plpgsql/src/Makefile
@@ -19,7 +19,7 @@ rpath =
 
 OBJS = pl_gram.o pl_handler.o pl_comp.o pl_exec.o pl_funcs.o pl_scanner.o
 
-DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql
+DATA = plpgsql.control plpgsql--1.1.sql plpgsql--1.0--1.1.sql plpgsql--unpackaged--1.0.sql
 
 all: all-lib
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 3749fac..ed540a9 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -86,6 +86,22 @@ typedef struct SimpleEcontextStackEntry
 static EState *shared_simple_eval_estate = NULL;
 static SimpleEcontextStackEntry *simple_econtext_stack = NULL;
 
+/*
+ * List of pointers and info of registered plugins.
+ */
+typedef struct PluginPtrEntry
+{
+	PLpgSQL_plugin *plugin_ptr;
+	struct PluginPtrEntry *next;
+} PluginPtrEntry;
+
+/*
+ * Allocated in TopMemoryContext
+ */
+static PluginPtrEntry *plugins = NULL;
+static int nplugins = 0;
+static int used_plugin_hook_types = 0;
+
 /
  * Local function forward declarations
  /
@@ -235,6 +251,18 @@ static char *format_expr_params(PLpgSQL_execstate *estate,
 const PLpgSQL_expr *expr);
 static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
 	   const PreparedParamsData *ppd);
+static void **get_plugin_info(PLpgSQL_execstate *estate, int plugin_number);
+
+
+/* Bits for used plugin callback types */
+#define PLUGIN_HOOK_TYPE_FUNC_SETUP	(1  0)
+#define PLUGIN_HOOK_TYPE_FUNC_BEG	(1  2)
+#define PLUGIN_HOOK_TYPE_FUNC_END	(1  3)
+#define PLUGIN_HOOK_TYPE_STMT_BEG	(1  4)
+#define PLUGIN_HOOK_TYPE_STMT_END	(1  5)
+
+#define EXEC_PLUGIN_HOOK_TYPE(type) \
+	((used_plugin_hook_types  (PLUGIN_HOOK_TYPE_ ## type)) == (PLUGIN_HOOK_TYPE_ ## type))
 
 
 /* --
@@ -331,10 +359,28 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	exec_set_found(estate, false);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_beg)
-		((*plugin_ptr)-func_beg) (estate, func);
+	if (EXEC_PLUGIN_HOOK_TYPE(FUNC_BEG))
+	{
+		PluginPtrEntry *pe;
+		int i;
+
+		Assert(plugins != NULL);
+
+		for (i = 0, pe = plugins; pe != NULL; pe = pe-next, i++)
+		{
+			PLpgSQL_plugin *pl_ptr = pe-plugin_ptr;
+
+			if (pl_ptr-func_beg)
+			{
+void *plugin_info;
+
+plugin_info = get_plugin_info(estate, i);
+(pl_ptr-func_beg) 

Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-01-17 Thread Pavel Stehule
2014/1/16 Marko Tiikkaja ma...@joh.to

 Hi Pavel,

 First of all, thanks for working on this!


 On 1/12/14, 8:58 PM, Pavel Stehule wrote:

 I still not happy with plugin_info - it is only per plugin now and should
 be per plugin and per function.


 I'm not sure I understand the point of plugin_info in the first place, but
 what would having a separate info per (plugin, function) achieve that can't
 be done otherwise?


First use case - I would to protect repeated call of plpgsql_check_function
in passive mode. Where I have to store information about successful first
start? It is related to the function instance, so function oid can be
ambiguous (for function with polymorphic parameters). When function
instance is destroyed, then this information should be destroyed. It is
impossible do this check from plugin. Second use case - attach session life
cycle plugin data with some function - for example for coverage
calculation. Inside plugin without function specific data you have to hold
a hash of all used function, and you have to search again and again. When
plpgsql hold this info in internal plpgsql function structures, then you
don't need search anything.




Regards

Pavel





 As for the current patch, I'd like to see improvements on a few things:

   1) It doesn't currently compile because of extra semicolons in the
  PLpgSQL_plugin struct.

   2) The previous comment above the same struct still talk about the
  rendezvous variable which is now gone.  The comment should be
  updated to reflect the new API.

   3) The same comment talks about how important it is to unregister a
  plugin if its _PG_fini() is ever called, but the current API does
  not support unregistering.  That should probably be added?  I'm not
  sure when _PG_fini() would be called.

   4) The comment  /* reserved for use by optional plugin */  seems a bit
  weird in its new context.


 Regards,
 Marko Tiikkaja



Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-01-16 Thread Marko Tiikkaja

Hi Pavel,

First of all, thanks for working on this!

On 1/12/14, 8:58 PM, Pavel Stehule wrote:

I still not happy with plugin_info - it is only per plugin now and should
be per plugin and per function.


I'm not sure I understand the point of plugin_info in the first place, 
but what would having a separate info per (plugin, function) achieve 
that can't be done otherwise?



As for the current patch, I'd like to see improvements on a few things:

  1) It doesn't currently compile because of extra semicolons in the
 PLpgSQL_plugin struct.

  2) The previous comment above the same struct still talk about the
 rendezvous variable which is now gone.  The comment should be
 updated to reflect the new API.

  3) The same comment talks about how important it is to unregister a
 plugin if its _PG_fini() is ever called, but the current API does
 not support unregistering.  That should probably be added?  I'm not
 sure when _PG_fini() would be called.

  4) The comment  /* reserved for use by optional plugin */  seems a bit
 weird in its new context.


Regards,
Marko Tiikkaja


--
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] proposal, patch: allow multiple plpgsql plugins

2014-01-12 Thread Marko Tiikkaja

On 1/9/14, 11:41 PM, Pavel Stehule wrote:

There are two basic questions:

b) will we support same API still - a reference on plugin_info in exec
state is a issue - described in patch.


Pardon my ignorance, but why does the plugin_info have to be in the 
executor state?  If we're going to change the API, can't we pass it 
directly to the callback function?  Am I missing something completely 
obvious? :-)



Regards,
Marko Tiikkaja


--
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] proposal, patch: allow multiple plpgsql plugins

2014-01-12 Thread Marko Tiikkaja

On 1/12/14, 5:33 PM, I wrote:

On 1/9/14, 11:41 PM, Pavel Stehule wrote:

There are two basic questions:

b) will we support same API still - a reference on plugin_info in exec
state is a issue - described in patch.


Pardon my ignorance, but why does the plugin_info have to be in the
executor state?  If we're going to change the API, can't we pass it
directly to the callback function?


Oh, I think I'm being stupid -- we'd only have to do what *if* we don't 
want to change the API?  Then my vote is for breaking the API.



Regards,
Marko Tiikkaja


--
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] proposal, patch: allow multiple plpgsql plugins

2014-01-12 Thread Pavel Stehule
2014/1/12 Marko Tiikkaja ma...@joh.to

 On 1/12/14, 5:33 PM, I wrote:

 On 1/9/14, 11:41 PM, Pavel Stehule wrote:

 There are two basic questions:

 b) will we support same API still - a reference on plugin_info in exec
 state is a issue - described in patch.


 Pardon my ignorance, but why does the plugin_info have to be in the
 executor state?  If we're going to change the API, can't we pass it
 directly to the callback function?


 Oh, I think I'm being stupid -- we'd only have to do what *if* we don't
 want to change the API?  Then my vote is for breaking the API.


yes. It is my vote too.

It is trouble - but support same API is really ugly - on second hand -
there are only few plpgsql plugins - and every plugin needs recompilation
for new mayor version and fixing will be easy.

Regards

Pavel Stehule




 Regards,
 Marko Tiikkaja



Re: [HACKERS] proposal, patch: allow multiple plpgsql plugins

2014-01-12 Thread Pavel Stehule
Hello

Updated version

I still not happy with plugin_info - it is only per plugin now and should
be per plugin and per function.

Regards

Pavel


2014/1/12 Pavel Stehule pavel.steh...@gmail.com




 2014/1/12 Marko Tiikkaja ma...@joh.to

 On 1/12/14, 5:33 PM, I wrote:

 On 1/9/14, 11:41 PM, Pavel Stehule wrote:

 There are two basic questions:

 b) will we support same API still - a reference on plugin_info in exec
 state is a issue - described in patch.


 Pardon my ignorance, but why does the plugin_info have to be in the
 executor state?  If we're going to change the API, can't we pass it
 directly to the callback function?


 Oh, I think I'm being stupid -- we'd only have to do what *if* we don't
 want to change the API?  Then my vote is for breaking the API.


 yes. It is my vote too.

 It is trouble - but support same API is really ugly - on second hand -
 there are only few plpgsql plugins - and every plugin needs recompilation
 for new mayor version and fixing will be easy.

 Regards

 Pavel Stehule




 Regards,
 Marko Tiikkaja



commit 56c89d4c41e4962d46fd83c6045b5827b51d9dfc
Author: Pavel Stehule pavel.steh...@gmail.com
Date:   Sun Jan 12 20:47:56 2014 +0100

cleaned version, break original plpgsql plugin API

diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index 852b0c7..37d17a8 100644
--- a/src/pl/plpgsql/src/Makefile
+++ b/src/pl/plpgsql/src/Makefile
@@ -19,7 +19,7 @@ rpath =
 
 OBJS = pl_gram.o pl_handler.o pl_comp.o pl_exec.o pl_funcs.o pl_scanner.o
 
-DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql
+DATA = plpgsql.control plpgsql--1.1.sql plpgsql--1.0--1.1.sql plpgsql--unpackaged--1.0.sql
 
 all: all-lib
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 3749fac..2b6b405 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -86,6 +86,22 @@ typedef struct SimpleEcontextStackEntry
 static EState *shared_simple_eval_estate = NULL;
 static SimpleEcontextStackEntry *simple_econtext_stack = NULL;
 
+/*
+ * List of pointers and info of registered plugins.
+ */
+typedef struct PluginPtrEntry
+{
+	PLpgSQL_plugin *plugin_ptr;
+	void *plugin_info;		/* reserved for use by optional plugin */
+	struct PluginPtrEntry *next;
+} PluginPtrEntry;
+
+/*
+ * Allocated in TopMemoryContext
+ */
+static PluginPtrEntry *plugins = NULL;
+static int used_plugin_hook_types = 0;
+
 /
  * Local function forward declarations
  /
@@ -236,6 +252,15 @@ static char *format_expr_params(PLpgSQL_execstate *estate,
 static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
 	   const PreparedParamsData *ppd);
 
+/* Bits for used plugin callback types */
+#define PLUGIN_HOOK_TYPE_FUNC_SETUP	(1  0)
+#define PLUGIN_HOOK_TYPE_FUNC_BEG	(1  2)
+#define PLUGIN_HOOK_TYPE_FUNC_END	(1  3)
+#define PLUGIN_HOOK_TYPE_STMT_BEG	(1  4)
+#define PLUGIN_HOOK_TYPE_STMT_END	(1  5)
+
+#define EXEC_PLUGIN_HOOK_TYPE(type) \
+	((used_plugin_hook_types  (PLUGIN_HOOK_TYPE_ ## type)) == (PLUGIN_HOOK_TYPE_ ## type))
 
 /* --
  * plpgsql_exec_function	Called by the call handler for
@@ -331,10 +356,22 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	exec_set_found(estate, false);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_beg)
-		((*plugin_ptr)-func_beg) (estate, func);
+	if (EXEC_PLUGIN_HOOK_TYPE(FUNC_BEG))
+	{
+		PluginPtrEntry *pe;
+
+		Assert(plugins != NULL);
+
+		for (pe = plugins; pe != NULL; pe = pe-next)
+		{
+			PLpgSQL_plugin *pl_ptr = pe-plugin_ptr;
+
+			if (pl_ptr-func_beg)
+(pl_ptr-func_beg) (estate, func, pe-plugin_info);
+		}
+	}
 
 	/*
 	 * Now call the toplevel block of statements
@@ -479,10 +516,22 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
 	estate.err_text = gettext_noop(during function exit);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_end)
-		((*plugin_ptr)-func_end) (estate, func);
+	if (EXEC_PLUGIN_HOOK_TYPE(FUNC_END))
+	{
+		PluginPtrEntry *pe;
+
+		Assert(plugins != NULL);
+
+		for (pe = plugins; pe != NULL; pe = pe-next)
+		{
+			PLpgSQL_plugin *pl_ptr = pe-plugin_ptr;
+
+			if (pl_ptr-func_end)
+(pl_ptr-func_end) (estate, func, pe-plugin_info);
+		}
+	}
 
 	/* Clean up any leftover temporary memory */
 	plpgsql_destroy_econtext(estate);
@@ -699,10 +748,22 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
 	exec_set_found(estate, false);
 
 	/*
-	 * Let the instrumentation plugin peek at this function
+	 * Let the instrumentation plugins peek at this function
 	 */
-	if (*plugin_ptr  (*plugin_ptr)-func_beg)
-		((*plugin_ptr)-func_beg) (estate, func);
+	if