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 (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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
