proposal, patch: allow multiple plpgsql plugins
Started by Pavel Stehuleabout 12 years ago14 messages
Hello
We talked about enhancing a plpgsql plugin API to support more active
plugins.
I wrote a prototype based on function plpgsql_register_plugin instead
rendezvous variable.
There are two basic questions:
a) will we support rendezvous variable still?
b) will we support same API still - a reference on plugin_info in exec
state is a issue - described in patch.
without a) a d b) we will break a current plugins little bit more than is
usual - not terrible hard to fix it. But without a) and b) a
implementation can be significantly cleaner.
Comments, notes?
Regards
Pavel
Attachments:
multiple-plpgsql-plugins.patchtext/x-patch; charset=US-ASCII; name=multiple-plpgsql-plugins.patchDownload
commit 406ee9d32dbb09385ec38bb6d89e8531cac1cd5f
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Thu Jan 9 23:32:30 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..fc7158e 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -86,6 +86,21 @@ 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;
+
/************************************************************
* Local function forward declarations
************************************************************/
@@ -236,6 +251,11 @@ static char *format_expr_params(PLpgSQL_execstate *estate,
static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
const PreparedParamsData *ppd);
+bool multi_plugin_func_setup = false;
+bool multi_plugin_func_beg = false;
+bool multi_plugin_func_end = false;
+bool multi_plugin_stmt_beg = false;
+bool multi_plugin_stmt_end = false;
/* ----------
* plpgsql_exec_function Called by the call handler for
@@ -336,6 +356,39 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
if (*plugin_ptr && (*plugin_ptr)->func_beg)
((*plugin_ptr)->func_beg) (&estate, func);
+ if (multi_plugin_func_beg)
+ {
+ PluginPtrEntry *plugin_entry;
+
+ Assert(plugins != NULL);
+
+ for (plugin_entry = plugins; plugin_entry != NULL;
+ plugin_entry = plugin_entry->next)
+ {
+ PLpgSQL_plugin *plugin_ptr = plugin_entry->plugin_ptr;
+
+ if (plugin_entry->plugin_ptr->func_beg)
+ {
+ void *plugin_info = estate.plugin_info;
+
+ /* save a plugin_info related single only plpgsql plugin */
+ PG_TRY();
+ {
+ estate.plugin_info = plugin_entry->plugin_info;
+ (plugin_ptr->func_beg) (&estate, func);
+ plugin_entry->plugin_info = estate.plugin_info;
+ estate.plugin_info = plugin_info;
+ }
+ PG_CATCH();
+ {
+ estate.plugin_info = plugin_info;
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+ }
+ }
+ }
+
/*
* Now call the toplevel block of statements
*/
@@ -484,6 +537,39 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
if (*plugin_ptr && (*plugin_ptr)->func_end)
((*plugin_ptr)->func_end) (&estate, func);
+ if (multi_plugin_func_end)
+ {
+ PluginPtrEntry *plugin_entry;
+
+ Assert(plugins != NULL);
+
+ for (plugin_entry = plugins; plugin_entry != NULL;
+ plugin_entry = plugin_entry->next)
+ {
+ PLpgSQL_plugin *plugin_ptr = plugin_entry->plugin_ptr;
+
+ if (plugin_entry->plugin_ptr->func_end)
+ {
+ void *plugin_info = estate.plugin_info;
+
+ /* save a plugin_info related single only plpgsql plugin */
+ PG_TRY();
+ {
+ estate.plugin_info = plugin_entry->plugin_info;
+ (plugin_ptr->func_end) (&estate, func);
+ plugin_entry->plugin_info = estate.plugin_info;
+ estate.plugin_info = plugin_info;
+ }
+ PG_CATCH();
+ {
+ estate.plugin_info = plugin_info;
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+ }
+ }
+ }
+
/* Clean up any leftover temporary memory */
plpgsql_destroy_econtext(&estate);
exec_eval_cleanup(&estate);
@@ -1393,6 +1479,39 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
if (*plugin_ptr && (*plugin_ptr)->stmt_beg)
((*plugin_ptr)->stmt_beg) (estate, stmt);
+ if (multi_plugin_stmt_beg)
+ {
+ PluginPtrEntry *plugin_entry;
+
+ Assert(plugins != NULL);
+
+ for (plugin_entry = plugins; plugin_entry != NULL;
+ plugin_entry = plugin_entry->next)
+ {
+ PLpgSQL_plugin *plugin_ptr = plugin_entry->plugin_ptr;
+
+ if (plugin_entry->plugin_ptr->stmt_beg)
+ {
+ void *plugin_info = estate->plugin_info;
+
+ /* save a plugin_info related single only plpgsql plugin */
+ PG_TRY();
+ {
+ estate->plugin_info = plugin_entry->plugin_info;
+ (plugin_ptr->stmt_beg) (estate, stmt);
+ plugin_entry->plugin_info = estate->plugin_info;
+ estate->plugin_info = plugin_info;
+ }
+ PG_CATCH();
+ {
+ estate->plugin_info = plugin_info;
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+ }
+ }
+ }
+
CHECK_FOR_INTERRUPTS();
switch ((enum PLpgSQL_stmt_types) stmt->cmd_type)
@@ -1498,6 +1617,39 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
if (*plugin_ptr && (*plugin_ptr)->stmt_end)
((*plugin_ptr)->stmt_end) (estate, stmt);
+ if (multi_plugin_stmt_end)
+ {
+ PluginPtrEntry *plugin_entry;
+
+ Assert(plugins != NULL);
+
+ for (plugin_entry = plugins; plugin_entry != NULL;
+ plugin_entry = plugin_entry->next)
+ {
+ PLpgSQL_plugin *plugin_ptr = plugin_entry->plugin_ptr;
+
+ if (plugin_entry->plugin_ptr->stmt_end)
+ {
+ void *plugin_info = estate->plugin_info;
+
+ /* save a plugin_info related single only plpgsql plugin */
+ PG_TRY();
+ {
+ estate->plugin_info = plugin_entry->plugin_info;
+ (plugin_ptr->stmt_end) (estate, stmt);
+ plugin_entry->plugin_info = estate->plugin_info;
+ estate->plugin_info = plugin_info;
+ }
+ PG_CATCH();
+ {
+ estate->plugin_info = plugin_info;
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+ }
+ }
+ }
+
estate->err_stmt = save_estmt;
return rc;
@@ -3181,6 +3333,42 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
if ((*plugin_ptr)->func_setup)
((*plugin_ptr)->func_setup) (estate, func);
}
+
+ if (plugins != NULL)
+ {
+ PluginPtrEntry *plugin_entry;
+
+ for (plugin_entry = plugins; plugin_entry != NULL;
+ plugin_entry = plugin_entry->next)
+ {
+ PLpgSQL_plugin *plugin_ptr = plugin_entry->plugin_ptr;
+
+ plugin_ptr->error_callback = plpgsql_exec_error_callback;
+ plugin_ptr->assign_expr = exec_assign_expr;
+
+ if (multi_plugin_func_setup)
+ {
+ void *plugin_info = estate->plugin_info;
+
+ Assert(plugin_ptr->func_setup != NULL);
+
+ /* save a plugin_info related single only plpgsql plugin */
+ PG_TRY();
+ {
+ estate->plugin_info = NULL;
+ (plugin_ptr->func_setup) (estate, func);
+ plugin_entry->plugin_info = plugin_info;
+ estate->plugin_info = plugin_info;
+ }
+ PG_CATCH();
+ {
+ estate->plugin_info = plugin_info;
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+ }
+ }
+ }
}
/* ----------
@@ -6632,3 +6820,38 @@ format_preparedparamsdata(PLpgSQL_execstate *estate,
return paramstr.data;
}
+
+/*
+ * Register a next plugin info
+ */
+void
+plpgsql_register_plugin_internal(PLpgSQL_plugin *plugin_ptr)
+{
+ PluginPtrEntry *new_entry;
+ PluginPtrEntry *last_entry;
+ MemoryContext old_cxt;
+
+ old_cxt = MemoryContextSwitchTo(TopMemoryContext);
+ new_entry = palloc0(sizeof(PluginPtrEntry));
+ MemoryContextSwitchTo(old_cxt);
+
+ /* search last registered plugin */
+ if (plugins != NULL)
+ {
+ for (last_entry = plugins; last_entry->next != NULL; last_entry = last_entry->next);
+ last_entry->next = new_entry;
+ }
+ else
+ plugins = new_entry;
+
+ if (plugin_ptr->func_setup != NULL)
+ multi_plugin_func_setup = true;
+ if (plugin_ptr->func_beg != NULL)
+ multi_plugin_func_beg = true;
+ if (plugin_ptr->func_end != NULL)
+ multi_plugin_func_end = true;
+ if (plugin_ptr->stmt_beg != NULL)
+ multi_plugin_stmt_beg = true;
+ if (plugin_ptr->stmt_end != NULL)
+ multi_plugin_stmt_end = true;
+}
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index f02203a..6f18711 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -384,3 +384,21 @@ plpgsql_validator(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+
+/* ----------
+ *
+ * register any plpgsql plugin
+ *
+ * ----------
+ */
+PG_FUNCTION_INFO_V1(plpgsql_register_plugin);
+
+Datum
+plpgsql_register_plugin(PG_FUNCTION_ARGS)
+{
+ PLpgSQL_plugin *plugin_ptr = (PLpgSQL_plugin *) PG_GETARG_POINTER(0);
+
+ plpgsql_register_plugin_internal(plugin_ptr);
+
+ PG_RETURN_VOID();
+}
diff --git a/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql b/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql
new file mode 100644
index 0000000..c7e9a62
--- /dev/null
+++ b/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql
@@ -0,0 +1,6 @@
+/* src/pl/plpgsql/src/plpgsql--1.0--1.1.sql */
+
+CREATE FUNCTION plpgsql_register_plugin(internal)
+RETURNS void
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT;
diff --git a/src/pl/plpgsql/src/plpgsql--1.0.sql b/src/pl/plpgsql/src/plpgsql--1.0.sql
deleted file mode 100644
index 5eeea56..0000000
--- a/src/pl/plpgsql/src/plpgsql--1.0.sql
+++ /dev/null
@@ -1,11 +0,0 @@
-/* src/pl/plpgsql/src/plpgsql--1.0.sql */
-
-/*
- * Currently, all the interesting stuff is done by CREATE LANGUAGE.
- * Later we will probably "dumb down" that command and put more of the
- * knowledge into this script.
- */
-
-CREATE PROCEDURAL LANGUAGE plpgsql;
-
-COMMENT ON PROCEDURAL LANGUAGE plpgsql IS 'PL/pgSQL procedural language';
diff --git a/src/pl/plpgsql/src/plpgsql--1.1.sql b/src/pl/plpgsql/src/plpgsql--1.1.sql
new file mode 100644
index 0000000..74aa4b9
--- /dev/null
+++ b/src/pl/plpgsql/src/plpgsql--1.1.sql
@@ -0,0 +1,16 @@
+/* src/pl/plpgsql/src/plpgsql--1.1.sql */
+
+/*
+ * Currently, all the interesting stuff is done by CREATE LANGUAGE.
+ * Later we will probably "dumb down" that command and put more of the
+ * knowledge into this script.
+ */
+
+CREATE PROCEDURAL LANGUAGE plpgsql;
+
+COMMENT ON PROCEDURAL LANGUAGE plpgsql IS 'PL/pgSQL procedural language';
+
+CREATE FUNCTION plpgsql_register_plugin(internal)
+RETURNS void
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT;
diff --git a/src/pl/plpgsql/src/plpgsql.control b/src/pl/plpgsql/src/plpgsql.control
index b320227..4c75c93 100644
--- a/src/pl/plpgsql/src/plpgsql.control
+++ b/src/pl/plpgsql/src/plpgsql.control
@@ -1,6 +1,6 @@
# plpgsql extension
comment = 'PL/pgSQL procedural language'
-default_version = '1.0'
+default_version = '1.1'
module_pathname = '$libdir/plpgsql'
relocatable = false
schema = pg_catalog
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index f3d1283..2d0e6f4 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -941,6 +941,7 @@ extern void _PG_init(void);
extern Datum plpgsql_call_handler(PG_FUNCTION_ARGS);
extern Datum plpgsql_inline_handler(PG_FUNCTION_ARGS);
extern Datum plpgsql_validator(PG_FUNCTION_ARGS);
+extern Datum plpgsql_register_plugin(PG_FUNCTION_ARGS);
/* ----------
* Functions in pl_exec.c
@@ -961,6 +962,7 @@ extern Oid exec_get_datum_type(PLpgSQL_execstate *estate,
extern void exec_get_datum_type_info(PLpgSQL_execstate *estate,
PLpgSQL_datum *datum,
Oid *typeid, int32 *typmod, Oid *collation);
+extern void plpgsql_register_plugin_internal(PLpgSQL_plugin *plugin_ptr);
/* ----------
* Functions for namespace handling in pl_funcs.c
Re: proposal, patch: allow multiple plpgsql plugins
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: proposal, patch: allow multiple plpgsql plugins
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: proposal, patch: allow multiple plpgsql plugins
2014/1/12 Marko Tiikkaja <marko@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
Show quoted text
Regards,
Marko Tiikkaja
Re: proposal, patch: allow multiple plpgsql plugins
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.stehule@gmail.com>
Show quoted text
2014/1/12 Marko Tiikkaja <marko@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
Attachments:
multiple-plpgsql-plugins-2014-01-12-01.patchtext/x-patch; charset=US-ASCII; name=multiple-plpgsql-plugins-2014-01-12-01.patchDownload
commit 56c89d4c41e4962d46fd83c6045b5827b51d9dfc
Author: Pavel Stehule <pavel.stehule@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 (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
@@ -768,10 +829,22 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
}
/*
- * 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);
@@ -831,10 +904,22 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
var->freeval = true;
/*
- * 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
@@ -865,10 +950,22 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
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);
@@ -1389,9 +1486,21 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
save_estmt = estate->err_stmt;
estate->err_stmt = stmt;
- /* Let the plugin know that we are about to execute this statement */
- if (*plugin_ptr && (*plugin_ptr)->stmt_beg)
- ((*plugin_ptr)->stmt_beg) (estate, stmt);
+ /* Let the plugins know that we are about to execute this statement */
+ if (EXEC_PLUGIN_HOOK_TYPE(STMT_BEG))
+ {
+ PluginPtrEntry *pe;
+
+ Assert(plugins != NULL);
+
+ for (pe = plugins; pe != NULL; pe = pe->next)
+ {
+ PLpgSQL_plugin *pl_ptr = pe->plugin_ptr;
+
+ if (pl_ptr->stmt_beg)
+ (pl_ptr->stmt_beg) (&estate, stmt, &pe->plugin_info);
+ }
+ }
CHECK_FOR_INTERRUPTS();
@@ -1495,8 +1604,20 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
}
/* Let the plugin know that we have finished executing this statement */
- if (*plugin_ptr && (*plugin_ptr)->stmt_end)
- ((*plugin_ptr)->stmt_end) (estate, stmt);
+ if (EXEC_PLUGIN_HOOK_TYPE(STMT_END))
+ {
+ PluginPtrEntry *pe;
+
+ Assert(plugins != NULL);
+
+ for (pe = plugins; pe != NULL; pe = pe->next)
+ {
+ PLpgSQL_plugin *pl_ptr = pe->plugin_ptr;
+
+ if (pl_ptr->stmt_end)
+ (pl_ptr->stmt_end) (&estate, stmt, &pe->plugin_info);
+ }
+ }
estate->err_stmt = save_estmt;
@@ -3160,8 +3281,6 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
estate->err_stmt = NULL;
estate->err_text = NULL;
- estate->plugin_info = NULL;
-
/*
* Create an EState and ExprContext for evaluation of simple expressions.
*/
@@ -3173,13 +3292,26 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
* pointers so it can call back into PL/pgSQL for doing things like
* variable assignments and stack traces
*/
- if (*plugin_ptr)
+ if (plugins)
{
- (*plugin_ptr)->error_callback = plpgsql_exec_error_callback;
- (*plugin_ptr)->assign_expr = exec_assign_expr;
+ PluginPtrEntry *pe;
+
+ Assert(plugins != NULL);
+
+ for (pe = plugins; pe != NULL; pe = pe->next)
+ {
+ PLpgSQL_plugin *pl_ptr = pe->plugin_ptr;
+
+ pl_ptr->error_callback = plpgsql_exec_error_callback;
+ pl_ptr->assign_expr = exec_assign_expr;
- if ((*plugin_ptr)->func_setup)
- ((*plugin_ptr)->func_setup) (estate, func);
+ if (EXEC_PLUGIN_HOOK_TYPE(FUNC_SETUP))
+ {
+ Assert(pl_ptr->func_setup);
+
+ (pl_ptr->func_setup) (estate, func, &pe->plugin_info);
+ }
+ }
}
}
@@ -6632,3 +6764,30 @@ format_preparedparamsdata(PLpgSQL_execstate *estate,
return paramstr.data;
}
+
+/*
+ * Register a next plugin info
+ */
+void
+plpgsql_register_plugin_internal(PLpgSQL_plugin *plugin_ptr)
+{
+ PluginPtrEntry *new_entry;
+ MemoryContext old_cxt;
+
+ old_cxt = MemoryContextSwitchTo(TopMemoryContext);
+ new_entry = palloc0(sizeof(PluginPtrEntry));
+ MemoryContextSwitchTo(old_cxt);
+
+ /*
+ * a order of plugins should not be important, so place new plugin as
+ * first item of plugin list. There are no necessary search end of list.
+ */
+ new_entry->next = plugins;
+ plugins = new_entry;
+
+ used_plugin_hook_types |= (plugin_ptr->func_setup ? PLUGIN_HOOK_TYPE_FUNC_SETUP : 0);
+ used_plugin_hook_types |= (plugin_ptr->func_beg ? PLUGIN_HOOK_TYPE_FUNC_BEG : 0);
+ used_plugin_hook_types |= (plugin_ptr->func_end ? PLUGIN_HOOK_TYPE_FUNC_END : 0);
+ used_plugin_hook_types |= (plugin_ptr->stmt_beg ? PLUGIN_HOOK_TYPE_STMT_BEG : 0);
+ used_plugin_hook_types |= (plugin_ptr->stmt_end ? PLUGIN_HOOK_TYPE_STMT_END : 0);
+}
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index f02203a..db6851e 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -39,10 +39,6 @@ int plpgsql_variable_conflict = PLPGSQL_RESOLVE_ERROR;
bool plpgsql_print_strict_params = false;
-/* Hook for plugins */
-PLpgSQL_plugin **plugin_ptr = NULL;
-
-
/*
* _PG_init() - library load-time initialization
*
@@ -82,9 +78,6 @@ _PG_init(void)
RegisterXactCallback(plpgsql_xact_cb, NULL);
RegisterSubXactCallback(plpgsql_subxact_cb, NULL);
- /* Set up a rendezvous point with optional instrumentation plugin */
- plugin_ptr = (PLpgSQL_plugin **) find_rendezvous_variable("PLpgSQL_plugin");
-
inited = true;
}
@@ -384,3 +377,21 @@ plpgsql_validator(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+
+/* ----------
+ *
+ * register any plpgsql plugin
+ *
+ * ----------
+ */
+PG_FUNCTION_INFO_V1(plpgsql_register_plugin);
+
+Datum
+plpgsql_register_plugin(PG_FUNCTION_ARGS)
+{
+ PLpgSQL_plugin *plugin_ptr = (PLpgSQL_plugin *) PG_GETARG_POINTER(0);
+
+ plpgsql_register_plugin_internal(plugin_ptr);
+
+ PG_RETURN_VOID();
+}
diff --git a/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql b/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql
new file mode 100644
index 0000000..c7e9a62
--- /dev/null
+++ b/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql
@@ -0,0 +1,6 @@
+/* src/pl/plpgsql/src/plpgsql--1.0--1.1.sql */
+
+CREATE FUNCTION plpgsql_register_plugin(internal)
+RETURNS void
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT;
diff --git a/src/pl/plpgsql/src/plpgsql--1.0.sql b/src/pl/plpgsql/src/plpgsql--1.0.sql
deleted file mode 100644
index 5eeea56..0000000
--- a/src/pl/plpgsql/src/plpgsql--1.0.sql
+++ /dev/null
@@ -1,11 +0,0 @@
-/* src/pl/plpgsql/src/plpgsql--1.0.sql */
-
-/*
- * Currently, all the interesting stuff is done by CREATE LANGUAGE.
- * Later we will probably "dumb down" that command and put more of the
- * knowledge into this script.
- */
-
-CREATE PROCEDURAL LANGUAGE plpgsql;
-
-COMMENT ON PROCEDURAL LANGUAGE plpgsql IS 'PL/pgSQL procedural language';
diff --git a/src/pl/plpgsql/src/plpgsql--1.1.sql b/src/pl/plpgsql/src/plpgsql--1.1.sql
new file mode 100644
index 0000000..74aa4b9
--- /dev/null
+++ b/src/pl/plpgsql/src/plpgsql--1.1.sql
@@ -0,0 +1,16 @@
+/* src/pl/plpgsql/src/plpgsql--1.1.sql */
+
+/*
+ * Currently, all the interesting stuff is done by CREATE LANGUAGE.
+ * Later we will probably "dumb down" that command and put more of the
+ * knowledge into this script.
+ */
+
+CREATE PROCEDURAL LANGUAGE plpgsql;
+
+COMMENT ON PROCEDURAL LANGUAGE plpgsql IS 'PL/pgSQL procedural language';
+
+CREATE FUNCTION plpgsql_register_plugin(internal)
+RETURNS void
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT;
diff --git a/src/pl/plpgsql/src/plpgsql.control b/src/pl/plpgsql/src/plpgsql.control
index b320227..4c75c93 100644
--- a/src/pl/plpgsql/src/plpgsql.control
+++ b/src/pl/plpgsql/src/plpgsql.control
@@ -1,6 +1,6 @@
# plpgsql extension
comment = 'PL/pgSQL procedural language'
-default_version = '1.0'
+default_version = '1.1'
module_pathname = '$libdir/plpgsql'
relocatable = false
schema = pg_catalog
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index f3d1283..b0c80c1 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -791,8 +791,6 @@ typedef struct PLpgSQL_execstate
/* status information for error context reporting */
PLpgSQL_stmt *err_stmt; /* current stmt */
const char *err_text; /* additional state info */
-
- void *plugin_info; /* reserved for use by optional plugin */
} PLpgSQL_execstate;
@@ -830,11 +828,16 @@ typedef struct PLpgSQL_execstate
typedef struct
{
/* Function pointers set up by the plugin */
- void (*func_setup) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*func_beg) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*func_end) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*stmt_beg) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt);
- void (*stmt_end) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt);
+ void (*func_setup) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*func_beg) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*func_end) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*stmt_beg) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt,
+ void **plugin_info;);
+ void (*stmt_end) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt,
+ void **plugin_info;);
/* Function pointers set by PL/pgSQL itself */
void (*error_callback) (void *arg);
@@ -894,8 +897,6 @@ extern char *plpgsql_error_funcname;
extern PLpgSQL_function *plpgsql_curr_compile;
extern MemoryContext compile_tmp_cxt;
-extern PLpgSQL_plugin **plugin_ptr;
-
/**********************************************************************
* Function declarations
**********************************************************************/
@@ -941,6 +942,7 @@ extern void _PG_init(void);
extern Datum plpgsql_call_handler(PG_FUNCTION_ARGS);
extern Datum plpgsql_inline_handler(PG_FUNCTION_ARGS);
extern Datum plpgsql_validator(PG_FUNCTION_ARGS);
+extern Datum plpgsql_register_plugin(PG_FUNCTION_ARGS);
/* ----------
* Functions in pl_exec.c
@@ -961,6 +963,7 @@ extern Oid exec_get_datum_type(PLpgSQL_execstate *estate,
extern void exec_get_datum_type_info(PLpgSQL_execstate *estate,
PLpgSQL_datum *datum,
Oid *typeid, int32 *typmod, Oid *collation);
+extern void plpgsql_register_plugin_internal(PLpgSQL_plugin *plugin_ptr);
/* ----------
* Functions for namespace handling in pl_funcs.c
Re: proposal, patch: allow multiple plpgsql plugins
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: proposal, patch: allow multiple plpgsql plugins
2014/1/16 Marko Tiikkaja <marko@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
Show quoted text
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: proposal, patch: allow multiple plpgsql plugins
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.stehule@gmail.com>:
Show quoted text
2014/1/16 Marko Tiikkaja <marko@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
Attachments:
multiple-plpgsql-plugins-2014-02-10-01.patchtext/x-patch; charset=US-ASCII; name=multiple-plpgsql-plugins-2014-02-10-01.patchDownload
commit eecd714579b3683b02a814b685b1d559ba0e0da8
Author: Pavel Stehule <pavel.stehule@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) (&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;
+
+ plugin_info = get_plugin_info(&estate, i);
+ (pl_ptr->func_end) (&estate, func, plugin_info);
+ }
+ }
+ }
/* Clean up any leftover temporary memory */
plpgsql_destroy_econtext(&estate);
@@ -699,10 +763,27 @@ 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 (EXEC_PLUGIN_HOOK_TYPE(FUNC_BEG))
+ {
+ 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;
+ void **plugin_info;
+
+ if (pl_ptr->func_beg)
+ {
+ plugin_info = get_plugin_info(&estate, i);
+ (pl_ptr->func_beg) (&estate, func, plugin_info);
+ }
+ }
+ }
/*
* Now call the toplevel block of statements
@@ -768,10 +849,27 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
}
/*
- * 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;
+ void **plugin_info;
+
+ if (pl_ptr->func_end)
+ {
+ plugin_info = get_plugin_info(&estate, i);
+ (pl_ptr->func_end) (&estate, func, plugin_info);
+ }
+ }
+ }
/* Clean up any leftover temporary memory */
plpgsql_destroy_econtext(&estate);
@@ -831,10 +929,28 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
var->freeval = true;
/*
- * 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; i++, pe = pe->next)
+ {
+ 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
@@ -865,10 +981,28 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
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;
+
+ plugin_info = get_plugin_info(&estate, i);
+ (pl_ptr->func_end) (&estate, func, plugin_info);
+ }
+ }
+ }
/* Clean up any leftover temporary memory */
plpgsql_destroy_econtext(&estate);
@@ -1389,9 +1523,27 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
save_estmt = estate->err_stmt;
estate->err_stmt = stmt;
- /* Let the plugin know that we are about to execute this statement */
- if (*plugin_ptr && (*plugin_ptr)->stmt_beg)
- ((*plugin_ptr)->stmt_beg) (estate, stmt);
+ /* Let the plugins know that we are about to execute this statement */
+ if (EXEC_PLUGIN_HOOK_TYPE(STMT_BEG))
+ {
+ 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->stmt_beg)
+ {
+ void **plugin_info;
+
+ plugin_info = get_plugin_info(estate, i);
+ (pl_ptr->stmt_beg) (&estate, stmt, plugin_info);
+ }
+ }
+ }
CHECK_FOR_INTERRUPTS();
@@ -1495,8 +1647,26 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
}
/* Let the plugin know that we have finished executing this statement */
- if (*plugin_ptr && (*plugin_ptr)->stmt_end)
- ((*plugin_ptr)->stmt_end) (estate, stmt);
+ if (EXEC_PLUGIN_HOOK_TYPE(STMT_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->stmt_end)
+ {
+ void **plugin_info;
+
+ plugin_info = get_plugin_info(estate, i);
+ (pl_ptr->stmt_end) (&estate, stmt, plugin_info);
+ }
+ }
+ }
estate->err_stmt = save_estmt;
@@ -3160,26 +3330,44 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
estate->err_stmt = NULL;
estate->err_text = NULL;
- estate->plugin_info = NULL;
-
/*
* Create an EState and ExprContext for evaluation of simple expressions.
*/
plpgsql_create_econtext(estate);
+ estate->plugin_info_vector = NULL;
+ estate->plugin_info_size = 0;
+
/*
* Let the plugin see this function before we initialize any local
* PL/pgSQL variables - note that we also give the plugin a few function
* pointers so it can call back into PL/pgSQL for doing things like
* variable assignments and stack traces
*/
- if (*plugin_ptr)
+ if (plugins)
{
- (*plugin_ptr)->error_callback = plpgsql_exec_error_callback;
- (*plugin_ptr)->assign_expr = exec_assign_expr;
+ 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;
+
+ pl_ptr->error_callback = plpgsql_exec_error_callback;
+ pl_ptr->assign_expr = exec_assign_expr;
+
+ if (EXEC_PLUGIN_HOOK_TYPE(FUNC_SETUP))
+ {
+ void **plugin_info;
+
+ Assert(pl_ptr->func_setup);
- if ((*plugin_ptr)->func_setup)
- ((*plugin_ptr)->func_setup) (estate, func);
+ plugin_info = get_plugin_info(estate, i);
+ (pl_ptr->func_setup) (estate, func, plugin_info);
+ }
+ }
}
}
@@ -6632,3 +6820,62 @@ format_preparedparamsdata(PLpgSQL_execstate *estate,
return paramstr.data;
}
+
+/*
+ * Register a next plugin info
+ */
+void
+plpgsql_register_plugin_internal(PLpgSQL_plugin *plugin_ptr)
+{
+ PluginPtrEntry *new_entry;
+ PluginPtrEntry *plugin;
+ MemoryContext old_cxt;
+
+ old_cxt = MemoryContextSwitchTo(TopMemoryContext);
+ new_entry = palloc0(sizeof(PluginPtrEntry));
+ MemoryContextSwitchTo(old_cxt);
+
+ /* Order of plugins is important - new plugins must be on end */
+ for (plugin = plugins; plugin->next != NULL; plugin = plugin->next);
+ plugin->next = new_entry;
+ nplugins++;
+
+ used_plugin_hook_types |= (plugin_ptr->func_setup ? PLUGIN_HOOK_TYPE_FUNC_SETUP : 0);
+ used_plugin_hook_types |= (plugin_ptr->func_beg ? PLUGIN_HOOK_TYPE_FUNC_BEG : 0);
+ used_plugin_hook_types |= (plugin_ptr->func_end ? PLUGIN_HOOK_TYPE_FUNC_END : 0);
+ used_plugin_hook_types |= (plugin_ptr->stmt_beg ? PLUGIN_HOOK_TYPE_STMT_BEG : 0);
+ used_plugin_hook_types |= (plugin_ptr->stmt_end ? PLUGIN_HOOK_TYPE_STMT_END : 0);
+}
+
+/*
+ * Ensure enough sized plugin_info_vector. This vector can be resized in runtime.
+ * It is unusual, but there are no reason, why we disallow it.
+ *
+ * Attention: plugins should not to cache plugin_info in private space. This pointer
+ * can be changed due possible repalloc.
+ */
+static void **
+get_plugin_info(PLpgSQL_execstate *estate, int plugin_number)
+{
+ int i;
+
+ if (estate->plugin_info_size >= plugin_number)
+ return (void **) &estate->plugin_info_vector[plugin_number - 1];
+
+ /*
+ * plugin_info_vector holds a pointers to function execution level
+ * plugin private memory. Designed implementation expects relatively
+ * immutable and small number of plugins. plugin_vector is resized
+ * to number of registered plugins. So usually plugin_info_vector
+ * is allocated only once.
+ */
+ estate->plugin_info_vector = repalloc(estate->plugin_info_vector,
+ nplugins * sizeof(void*));
+
+ for (i = estate->plugin_info_size; i < nplugins - 1; i++)
+ estate->plugin_info_vector[i] = NULL;
+
+ estate->plugin_info_size = nplugins;
+
+ return (void **) &estate->plugin_info_vector[plugin_number - 1];
+}
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index f02203a..db6851e 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -39,10 +39,6 @@ int plpgsql_variable_conflict = PLPGSQL_RESOLVE_ERROR;
bool plpgsql_print_strict_params = false;
-/* Hook for plugins */
-PLpgSQL_plugin **plugin_ptr = NULL;
-
-
/*
* _PG_init() - library load-time initialization
*
@@ -82,9 +78,6 @@ _PG_init(void)
RegisterXactCallback(plpgsql_xact_cb, NULL);
RegisterSubXactCallback(plpgsql_subxact_cb, NULL);
- /* Set up a rendezvous point with optional instrumentation plugin */
- plugin_ptr = (PLpgSQL_plugin **) find_rendezvous_variable("PLpgSQL_plugin");
-
inited = true;
}
@@ -384,3 +377,21 @@ plpgsql_validator(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+
+/* ----------
+ *
+ * register any plpgsql plugin
+ *
+ * ----------
+ */
+PG_FUNCTION_INFO_V1(plpgsql_register_plugin);
+
+Datum
+plpgsql_register_plugin(PG_FUNCTION_ARGS)
+{
+ PLpgSQL_plugin *plugin_ptr = (PLpgSQL_plugin *) PG_GETARG_POINTER(0);
+
+ plpgsql_register_plugin_internal(plugin_ptr);
+
+ PG_RETURN_VOID();
+}
diff --git a/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql b/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql
new file mode 100644
index 0000000..c7e9a62
--- /dev/null
+++ b/src/pl/plpgsql/src/plpgsql--1.0--1.1.sql
@@ -0,0 +1,6 @@
+/* src/pl/plpgsql/src/plpgsql--1.0--1.1.sql */
+
+CREATE FUNCTION plpgsql_register_plugin(internal)
+RETURNS void
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT;
diff --git a/src/pl/plpgsql/src/plpgsql--1.1.sql b/src/pl/plpgsql/src/plpgsql--1.1.sql
new file mode 100644
index 0000000..74aa4b9
--- /dev/null
+++ b/src/pl/plpgsql/src/plpgsql--1.1.sql
@@ -0,0 +1,16 @@
+/* src/pl/plpgsql/src/plpgsql--1.1.sql */
+
+/*
+ * Currently, all the interesting stuff is done by CREATE LANGUAGE.
+ * Later we will probably "dumb down" that command and put more of the
+ * knowledge into this script.
+ */
+
+CREATE PROCEDURAL LANGUAGE plpgsql;
+
+COMMENT ON PROCEDURAL LANGUAGE plpgsql IS 'PL/pgSQL procedural language';
+
+CREATE FUNCTION plpgsql_register_plugin(internal)
+RETURNS void
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT;
diff --git a/src/pl/plpgsql/src/plpgsql.control b/src/pl/plpgsql/src/plpgsql.control
index b320227..4c75c93 100644
--- a/src/pl/plpgsql/src/plpgsql.control
+++ b/src/pl/plpgsql/src/plpgsql.control
@@ -1,6 +1,6 @@
# plpgsql extension
comment = 'PL/pgSQL procedural language'
-default_version = '1.0'
+default_version = '1.1'
module_pathname = '$libdir/plpgsql'
relocatable = false
schema = pg_catalog
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index f3d1283..6494fb4 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -792,7 +792,8 @@ typedef struct PLpgSQL_execstate
PLpgSQL_stmt *err_stmt; /* current stmt */
const char *err_text; /* additional state info */
- void *plugin_info; /* reserved for use by optional plugin */
+ void **plugin_info_vector; /* array of plugin info ptrs */
+ int plugin_info_size; /* size of plugin_info_vector */
} PLpgSQL_execstate;
@@ -830,11 +831,16 @@ typedef struct PLpgSQL_execstate
typedef struct
{
/* Function pointers set up by the plugin */
- void (*func_setup) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*func_beg) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*func_end) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*stmt_beg) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt);
- void (*stmt_end) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt);
+ void (*func_setup) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*func_beg) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*func_end) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*stmt_beg) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt,
+ void **plugin_info;);
+ void (*stmt_end) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt,
+ void **plugin_info;);
/* Function pointers set by PL/pgSQL itself */
void (*error_callback) (void *arg);
@@ -894,8 +900,6 @@ extern char *plpgsql_error_funcname;
extern PLpgSQL_function *plpgsql_curr_compile;
extern MemoryContext compile_tmp_cxt;
-extern PLpgSQL_plugin **plugin_ptr;
-
/**********************************************************************
* Function declarations
**********************************************************************/
@@ -941,6 +945,7 @@ extern void _PG_init(void);
extern Datum plpgsql_call_handler(PG_FUNCTION_ARGS);
extern Datum plpgsql_inline_handler(PG_FUNCTION_ARGS);
extern Datum plpgsql_validator(PG_FUNCTION_ARGS);
+extern Datum plpgsql_register_plugin(PG_FUNCTION_ARGS);
/* ----------
* Functions in pl_exec.c
@@ -961,6 +966,7 @@ extern Oid exec_get_datum_type(PLpgSQL_execstate *estate,
extern void exec_get_datum_type_info(PLpgSQL_execstate *estate,
PLpgSQL_datum *datum,
Oid *typeid, int32 *typmod, Oid *collation);
+extern void plpgsql_register_plugin_internal(PLpgSQL_plugin *plugin_ptr);
/* ----------
* Functions for namespace handling in pl_funcs.c
Re: proposal, patch: allow multiple plpgsql plugins
Hello Marko
2014-01-16 23:54 GMT+01:00 Marko Tiikkaja <marko@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
Show quoted text
4) The comment /* reserved for use by optional plugin */ seems a bit
weird in its new context.Regards,
Marko Tiikkaja
Attachments:
multiple-plpgsql-plugins-2014-02-10-02.patchtext/x-patch; charset=US-ASCII; name=multiple-plpgsql-plugins-2014-02-10-02.patchDownload
commit bf0820786f08b08812bba3d86233cbac30922054
Author: Pavel Stehule <pavel.stehule@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;
+
+ plugin_info = get_plugin_info(&estate, i);
+ (pl_ptr->func_end) (&estate, func, plugin_info);
+ }
+ }
+ }
/* Clean up any leftover temporary memory */
plpgsql_destroy_econtext(&estate);
@@ -699,10 +763,27 @@ 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 (EXEC_PLUGIN_HOOK_TYPE(FUNC_BEG))
+ {
+ 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;
+ void **plugin_info;
+
+ if (pl_ptr->func_beg)
+ {
+ plugin_info = get_plugin_info(&estate, i);
+ (pl_ptr->func_beg) (&estate, func, plugin_info);
+ }
+ }
+ }
/*
* Now call the toplevel block of statements
@@ -768,10 +849,27 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
}
/*
- * 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;
+ void **plugin_info;
+
+ if (pl_ptr->func_end)
+ {
+ plugin_info = get_plugin_info(&estate, i);
+ (pl_ptr->func_end) (&estate, func, plugin_info);
+ }
+ }
+ }
/* Clean up any leftover temporary memory */
plpgsql_destroy_econtext(&estate);
@@ -831,10 +929,28 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
var->freeval = true;
/*
- * 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; i++, pe = pe->next)
+ {
+ 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
@@ -865,10 +981,28 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
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;
+
+ plugin_info = get_plugin_info(&estate, i);
+ (pl_ptr->func_end) (&estate, func, plugin_info);
+ }
+ }
+ }
/* Clean up any leftover temporary memory */
plpgsql_destroy_econtext(&estate);
@@ -1389,9 +1523,27 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
save_estmt = estate->err_stmt;
estate->err_stmt = stmt;
- /* Let the plugin know that we are about to execute this statement */
- if (*plugin_ptr && (*plugin_ptr)->stmt_beg)
- ((*plugin_ptr)->stmt_beg) (estate, stmt);
+ /* Let the plugins know that we are about to execute this statement */
+ if (EXEC_PLUGIN_HOOK_TYPE(STMT_BEG))
+ {
+ 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->stmt_beg)
+ {
+ void **plugin_info;
+
+ plugin_info = get_plugin_info(estate, i);
+ (pl_ptr->stmt_beg) (&estate, stmt, plugin_info);
+ }
+ }
+ }
CHECK_FOR_INTERRUPTS();
@@ -1495,8 +1647,26 @@ exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
}
/* Let the plugin know that we have finished executing this statement */
- if (*plugin_ptr && (*plugin_ptr)->stmt_end)
- ((*plugin_ptr)->stmt_end) (estate, stmt);
+ if (EXEC_PLUGIN_HOOK_TYPE(STMT_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->stmt_end)
+ {
+ void **plugin_info;
+
+ plugin_info = get_plugin_info(estate, i);
+ (pl_ptr->stmt_end) (&estate, stmt, plugin_info);
+ }
+ }
+ }
estate->err_stmt = save_estmt;
@@ -3160,26 +3330,44 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
estate->err_stmt = NULL;
estate->err_text = NULL;
- estate->plugin_info = NULL;
-
/*
* Create an EState and ExprContext for evaluation of simple expressions.
*/
plpgsql_create_econtext(estate);
+ estate->plugin_info_vector = NULL;
+ estate->plugin_info_size = 0;
+
/*
* Let the plugin see this function before we initialize any local
* PL/pgSQL variables - note that we also give the plugin a few function
* pointers so it can call back into PL/pgSQL for doing things like
* variable assignments and stack traces
*/
- if (*plugin_ptr)
+ if (plugins)
{
- (*plugin_ptr)->error_callback = plpgsql_exec_error_callback;
- (*plugin_ptr)->assign_expr = exec_assign_expr;
+ 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;
+
+ pl_ptr->error_callback = plpgsql_exec_error_callback;
+ pl_ptr->assign_expr = exec_assign_expr;
+
+ if (EXEC_PLUGIN_HOOK_TYPE(FUNC_SETUP))
+ {
+ void **plugin_info;
+
+ Assert(pl_ptr->func_setup);
- if ((*plugin_ptr)->func_setup)
- ((*plugin_ptr)->func_setup) (estate, func);
+ plugin_info = get_plugin_info(estate, i);
+ (pl_ptr->func_setup) (estate, func, plugin_info);
+ }
+ }
}
}
@@ -6632,3 +6820,62 @@ format_preparedparamsdata(PLpgSQL_execstate *estate,
return paramstr.data;
}
+
+/*
+ * Register a next plugin info
+ */
+void
+plpgsql_register_plugin_internal(PLpgSQL_plugin *plugin_ptr)
+{
+ PluginPtrEntry *new_entry;
+ PluginPtrEntry *plugin;
+ MemoryContext old_cxt;
+
+ old_cxt = MemoryContextSwitchTo(TopMemoryContext);
+ new_entry = palloc0(sizeof(PluginPtrEntry));
+ MemoryContextSwitchTo(old_cxt);
+
+ /* Order of plugins is important - new plugins must be on end */
+ for (plugin = plugins; plugin->next != NULL; plugin = plugin->next);
+ plugin->next = new_entry;
+ nplugins++;
+
+ used_plugin_hook_types |= (plugin_ptr->func_setup ? PLUGIN_HOOK_TYPE_FUNC_SETUP : 0);
+ used_plugin_hook_types |= (plugin_ptr->func_beg ? PLUGIN_HOOK_TYPE_FUNC_BEG : 0);
+ used_plugin_hook_types |= (plugin_ptr->func_end ? PLUGIN_HOOK_TYPE_FUNC_END : 0);
+ used_plugin_hook_types |= (plugin_ptr->stmt_beg ? PLUGIN_HOOK_TYPE_STMT_BEG : 0);
+ used_plugin_hook_types |= (plugin_ptr->stmt_end ? PLUGIN_HOOK_TYPE_STMT_END : 0);
+}
+
+/*
+ * Ensure enough sized plugin_info_vector. This vector can be resized in runtime.
+ * It is unusual, but there are no reason, why we disallow it.
+ *
+ * Attention: plugins should not to cache plugin_info in private space. This pointer
+ * can be changed due possible repalloc.
+ */
+static void **
+get_plugin_info(PLpgSQL_execstate *estate, int plugin_number)
+{
+ int i;
+
+ if (estate->plugin_info_size >= plugin_number)
+ return (void **) &estate->plugin_info_vector[plugin_number - 1];
+
+ /*
+ * plugin_info_vector holds a pointers to function execution level
+ * plugin private memory. Designed implementation expects relatively
+ * immutable and small number of plugins. plugin_vector is resized
+ * to number of registered plugins. So usually plugin_info_vector
+ * is allocated only once.
+ */
+ estate->plugin_info_vector = repalloc(estate->plugin_info_vector,
+ nplugins * sizeof(void*));
+
+ for (i = estate->plugin_info_size; i < nplugins - 1; i++)
+ estate->plugin_info_vector[i] = NULL;
+
+ estate->plugin_info_size = nplugins;
+
+ return (void **) &estate->plugin_info_vector[plugin_number - 1];
+}
diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index f02203a..db6851e 100644
--- a/src/pl/plpgsql/src/pl_handler.c
+++ b/src/pl/plpgsql/src/pl_handler.c
@@ -39,10 +39,6 @@ int plpgsql_variable_conflict = PLPGSQL_RESOLVE_ERROR;
bool plpgsql_print_strict_params = false;
-/* Hook for plugins */
-PLpgSQL_plugin **plugin_ptr = NULL;
-
-
/*
* _PG_init() - library load-time initialization
*
@@ -82,9 +78,6 @@ _PG_init(void)
RegisterXactCallback(plpgsql_xact_cb, NULL);
RegisterSubXactCallback(plpgsql_subxact_cb, NULL);
- /* Set up a rendezvous point with optional instrumentation plugin */
- plugin_ptr = (PLpgSQL_plugin **) find_rendezvous_variable("PLpgSQL_plugin");
-
inited = true;
}
@@ -384,3 +377,21 @@ plpgsql_validator(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+
+/* ----------
+ *
+ * register any plpgsql plugin
+ *
+ * ----------
+ */
+PG_FUNCTION_INFO_V1(plpgsql_register_plugin);
+
+Datum
+plpgsql_register_plugin(PG_FUNCTION_ARGS)
+{
+ PLpgSQL_plugin *plugin_ptr = (PLpgSQL_plugin *) PG_GETARG_POINTER(0);
+
+ plpgsql_register_plugin_internal(plugin_ptr);
+
+ PG_RETURN_VOID();
+}
diff --git a/src/pl/plpgsql/src/plpgsql.control b/src/pl/plpgsql/src/plpgsql.control
index b320227..4c75c93 100644
--- a/src/pl/plpgsql/src/plpgsql.control
+++ b/src/pl/plpgsql/src/plpgsql.control
@@ -1,6 +1,6 @@
# plpgsql extension
comment = 'PL/pgSQL procedural language'
-default_version = '1.0'
+default_version = '1.1'
module_pathname = '$libdir/plpgsql'
relocatable = false
schema = pg_catalog
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index f3d1283..cac9b9a 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -792,18 +792,16 @@ typedef struct PLpgSQL_execstate
PLpgSQL_stmt *err_stmt; /* current stmt */
const char *err_text; /* additional state info */
- void *plugin_info; /* reserved for use by optional plugin */
+ void **plugin_info_vector; /* array of plugin info ptrs */
+ int plugin_info_size; /* size of plugin_info_vector */
} PLpgSQL_execstate;
/*
* A PLpgSQL_plugin structure represents an instrumentation plugin.
- * To instrument PL/pgSQL, a plugin library must access the rendezvous
- * variable "PLpgSQL_plugin" and set it to point to a PLpgSQL_plugin struct.
* Typically the struct could just be static data in the plugin library.
- * We expect that a plugin would do this at library load time (_PG_init()).
- * It must also be careful to set the rendezvous variable back to NULL
- * if it is unloaded (_PG_fini()).
+ * We expect that a plugin would do this at library load time (_PG_init())
+ * with call plpgsql_register_plugin.
*
* This structure is basically a collection of function pointers --- at
* various interesting points in pl_exec.c, we call these functions
@@ -830,11 +828,16 @@ typedef struct PLpgSQL_execstate
typedef struct
{
/* Function pointers set up by the plugin */
- void (*func_setup) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*func_beg) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*func_end) (PLpgSQL_execstate *estate, PLpgSQL_function *func);
- void (*stmt_beg) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt);
- void (*stmt_end) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt);
+ void (*func_setup) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*func_beg) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*func_end) (PLpgSQL_execstate *estate, PLpgSQL_function *func,
+ void **plugin_info;);
+ void (*stmt_beg) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt,
+ void **plugin_info;);
+ void (*stmt_end) (PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt,
+ void **plugin_info;);
/* Function pointers set by PL/pgSQL itself */
void (*error_callback) (void *arg);
@@ -894,8 +897,6 @@ extern char *plpgsql_error_funcname;
extern PLpgSQL_function *plpgsql_curr_compile;
extern MemoryContext compile_tmp_cxt;
-extern PLpgSQL_plugin **plugin_ptr;
-
/**********************************************************************
* Function declarations
**********************************************************************/
@@ -941,6 +942,7 @@ extern void _PG_init(void);
extern Datum plpgsql_call_handler(PG_FUNCTION_ARGS);
extern Datum plpgsql_inline_handler(PG_FUNCTION_ARGS);
extern Datum plpgsql_validator(PG_FUNCTION_ARGS);
+extern Datum plpgsql_register_plugin(PG_FUNCTION_ARGS);
/* ----------
* Functions in pl_exec.c
@@ -961,6 +963,7 @@ extern Oid exec_get_datum_type(PLpgSQL_execstate *estate,
extern void exec_get_datum_type_info(PLpgSQL_execstate *estate,
PLpgSQL_datum *datum,
Oid *typeid, int32 *typmod, Oid *collation);
+extern void plpgsql_register_plugin_internal(PLpgSQL_plugin *plugin_ptr);
/* ----------
* Functions for namespace handling in pl_funcs.c
Re: proposal, patch: allow multiple plpgsql plugins
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: proposal, patch: allow multiple plpgsql plugins
Hi
2014-03-02 19:59 GMT+01:00 Marko Tiikkaja <marko@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.oI'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.
or you can add a reference on plpgsql to your Makefile
Regards
Pavel
Show quoted text
Regards,
Marko Tiikkaja
Re: proposal, patch: allow multiple plpgsql plugins
On 3/2/14, 8:47 PM, Pavel Stehule wrote:
2014-03-02 19:59 GMT+01:00 Marko Tiikkaja <marko@joh.to>:
Undefined symbols for architecture x86_64:
"_plpgsql_register_plugin", referenced from:
__PG_init in plpgtest.oI'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.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: proposal, patch: allow multiple plpgsql plugins
Dne 2. 3. 2014 21:55 "Marko Tiikkaja" <marko@joh.to> napsal(a):
On 3/2/14, 8:47 PM, Pavel Stehule wrote:
2014-03-02 19:59 GMT+01:00 Marko Tiikkaja <marko@joh.to>:
Undefined symbols for architecture x86_64:
"_plpgsql_register_plugin", referenced from:
__PG_init in plpgtest.oI'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
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.soI 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: proposal, patch: allow multiple plpgsql plugins
2014-03-03 6:09 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Dne 2. 3. 2014 21:55 "Marko Tiikkaja" <marko@joh.to> napsal(a):
On 3/2/14, 8:47 PM, Pavel Stehule wrote:
2014-03-02 19:59 GMT+01:00 Marko Tiikkaja <marko@joh.to>:
Undefined symbols for architecture x86_64:
"_plpgsql_register_plugin", referenced from:
__PG_init in plpgtest.oI'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
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_pluginReferenced from: /usr/local/pgsql/lib/plpgtest.so
Expected in: flat namespace
in /usr/local/pgsql/lib/plpgtest.soI even tried putting plpgsql.so before it in the list, but no go.
Regards,
Marko TiikkajaIn 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
Show quoted text
Regards
pavel