From 46211da1b84bc3537e799ee1126098e71c2428e8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 17 Jan 2011 21:46:36 +0200
Subject: [PATCH] Use HTABs instead of Python dictionary objects to cache
procedures
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Two separate hash tables are used for regular procedures and for
trigger procedures, since the way trigger procedures work is quite
different from normal stored procedures. Change the signatures of
PLy_procedure_{get,create} to accept the function OID and a Boolean
flag indicating whether it's a trigger. This should make implementing
a PL/Python validator easier.
Using HTABs instead of Python dictionaries makes error recovery
easier, and allows for procedures to be cached based on their OIDs,
not their names. It also allows getting rid of the PyCObject field
that used to hold a pointer to PLyProcedure, since PyCObjects are
deprecated in Python 2.7 and replaced by Capsules in Python 3.
Jan Urbański
---
src/pl/plpython/plpython.c | 204 ++++++++++++++++++++-----------------
1 file changed, 110 insertions(+), 94 deletions(-)
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index d3b48ae6752..c25db9344f6 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -102,6 +102,7 @@ typedef int Py_ssize_t;
#include "parser/parse_type.h"
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
+#include "utils/hsearch.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/syscache.h"
@@ -214,11 +215,17 @@ typedef struct PLyProcedure
PyObject *code; /* compiled procedure code */
PyObject *statics; /* data saved across calls, local scope */
PyObject *globals; /* data saved across calls, global scope */
- PyObject *me; /* PyCObject containing pointer to this
- * PLyProcedure */
} PLyProcedure;
+/* the procedure cache entry */
+typedef struct PLyProcedureEntry
+{
+ Oid fn_oid; /* hash key */
+ PLyProcedure *proc;
+} PLyProcedureEntry;
+
+
/* Python objects */
typedef struct PLyPlanObject
{
@@ -311,11 +318,10 @@ static HeapTuple PLy_modify_tuple(PLyProcedure *, PyObject *,
static PyObject *PLy_procedure_call(PLyProcedure *, char *, PyObject *);
-static PLyProcedure *PLy_procedure_get(FunctionCallInfo fcinfo,
- Oid tgreloid);
+static PLyProcedure *PLy_procedure_get(Oid fn_oid, bool is_trigger);
-static PLyProcedure *PLy_procedure_create(HeapTuple procTup, Oid tgreloid,
- char *key);
+static PLyProcedure *PLy_procedure_create(HeapTuple procTup,
+ Oid fn_oid, bool is_trigger);
static void PLy_procedure_compile(PLyProcedure *, const char *);
static char *PLy_procedure_munge_source(const char *, const char *);
@@ -373,7 +379,8 @@ static ErrorData *PLy_error_in_progress = NULL;
static PyObject *PLy_interp_globals = NULL;
static PyObject *PLy_interp_safe_globals = NULL;
-static PyObject *PLy_procedure_cache = NULL;
+static HTAB *PLy_procedure_cache = NULL;
+static HTAB *PLy_trigger_cache = NULL;
/* Python exceptions */
static PyObject *PLy_exc_error = NULL;
@@ -444,7 +451,6 @@ plpython_call_handler(PG_FUNCTION_ARGS)
{
Datum retval;
PLyProcedure *save_curr_proc;
- PLyProcedure *volatile proc = NULL;
ErrorContextCallback plerrcontext;
if (SPI_connect() != SPI_OK_CONNECT)
@@ -461,20 +467,20 @@ plpython_call_handler(PG_FUNCTION_ARGS)
PG_TRY();
{
+ PLyProcedure *proc;
+
if (CALLED_AS_TRIGGER(fcinfo))
{
- TriggerData *tdata = (TriggerData *) fcinfo->context;
HeapTuple trv;
- proc = PLy_procedure_get(fcinfo,
- RelationGetRelid(tdata->tg_relation));
+ proc = PLy_procedure_get(fcinfo->flinfo->fn_oid, true);
PLy_curr_procedure = proc;
trv = PLy_trigger_handler(fcinfo, proc);
retval = PointerGetDatum(trv);
}
else
{
- proc = PLy_procedure_get(fcinfo, InvalidOid);
+ proc = PLy_procedure_get(fcinfo->flinfo->fn_oid, false);
PLy_curr_procedure = proc;
retval = PLy_function_handler(fcinfo, proc);
}
@@ -482,11 +488,6 @@ plpython_call_handler(PG_FUNCTION_ARGS)
PG_CATCH();
{
PLy_curr_procedure = save_curr_proc;
- if (proc)
- {
- /* note: Py_DECREF needs braces around it, as of 2003/08 */
- Py_DECREF(proc->me);
- }
PyErr_Clear();
PG_RE_THROW();
}
@@ -497,8 +498,6 @@ plpython_call_handler(PG_FUNCTION_ARGS)
PLy_curr_procedure = save_curr_proc;
- Py_DECREF(proc->me);
-
return retval;
}
@@ -575,6 +574,22 @@ PLy_trigger_handler(FunctionCallInfo fcinfo, PLyProcedure *proc)
HeapTuple rv = NULL;
PyObject *volatile plargs = NULL;
PyObject *volatile plrv = NULL;
+ TriggerData *tdata;
+
+ Assert(CALLED_AS_TRIGGER(fcinfo));
+
+ /*
+ * Input/output conversion for trigger tuples. Use the result
+ * TypeInfo variable to store the tuple conversion info. We do
+ * this over again on each call to cover the possibility that the
+ * relation's tupdesc changed since the trigger was last called.
+ * PLy_input_tuple_funcs and PLy_output_tuple_funcs are
+ * responsible for not doing repetitive work.
+ */
+ tdata = (TriggerData *) fcinfo->context;
+
+ PLy_input_tuple_funcs(&(proc->result), tdata->tg_relation->rd_att);
+ PLy_output_tuple_funcs(&(proc->result), tdata->tg_relation->rd_att);
PG_TRY();
{
@@ -1285,6 +1300,19 @@ PLy_function_delete_args(PLyProcedure *proc)
PyDict_DelItemString(proc->globals, proc->argnames[i]);
}
+/*
+ * Decide whether a cached PLyProcedure struct is still valid
+ */
+static bool
+PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
+{
+ Assert(proc != NULL);
+
+ /* If the pg_proc tuple has changed, it's not valid */
+ return (proc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
+ ItemPointerEquals(&proc->fn_tid, &procTup->t_self));
+}
+
/*
* PLyProcedure functions
@@ -1296,73 +1324,63 @@ PLy_function_delete_args(PLyProcedure *proc)
* function calls.
*/
static PLyProcedure *
-PLy_procedure_get(FunctionCallInfo fcinfo, Oid tgreloid)
+PLy_procedure_get(Oid fn_oid, bool is_trigger)
{
- Oid fn_oid;
HeapTuple procTup;
- char key[128];
- PyObject *plproc;
- PLyProcedure *proc = NULL;
- int rv;
+ PLyProcedureEntry *entry;
+ bool found;
- fn_oid = fcinfo->flinfo->fn_oid;
procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fn_oid));
if (!HeapTupleIsValid(procTup))
elog(ERROR, "cache lookup failed for function %u", fn_oid);
- rv = snprintf(key, sizeof(key), "%u_%u", fn_oid, tgreloid);
- if (rv >= sizeof(key) || rv < 0)
- elog(ERROR, "key too long");
-
- plproc = PyDict_GetItemString(PLy_procedure_cache, key);
+ /* Look for the function in the corresponding cache */
+ if (is_trigger)
+ entry = hash_search(PLy_trigger_cache,
+ &fn_oid, HASH_ENTER, &found);
+ else
+ entry = hash_search(PLy_procedure_cache,
+ &fn_oid, HASH_ENTER, &found);
- if (plproc != NULL)
+ PG_TRY();
{
- Py_INCREF(plproc);
- if (!PyCObject_Check(plproc))
- elog(FATAL, "expected a PyCObject, didn't get one");
-
- proc = PyCObject_AsVoidPtr(plproc);
- if (!proc)
- PLy_elog(ERROR, "PyCObject_AsVoidPtr() failed");
- if (proc->me != plproc)
- elog(FATAL, "proc->me != plproc");
- /* did we find an up-to-date cache entry? */
- if (proc->fn_xmin != HeapTupleHeaderGetXmin(procTup->t_data) ||
- !ItemPointerEquals(&proc->fn_tid, &procTup->t_self))
+ if (!found)
+ {
+ /* Haven't found it, create a new cache entry */
+ entry->proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
+ }
+ else if (!PLy_procedure_valid(entry->proc, procTup))
{
- Py_DECREF(plproc);
- proc = NULL;
+ /* Found it, but it's invalid, free and reuse the cache entry */
+ PLy_procedure_delete(entry->proc);
+ PLy_free(entry->proc);
+ entry->proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
}
+ /* Found it and it's valid, it's fine to use it */
}
-
- if (proc == NULL)
- proc = PLy_procedure_create(procTup, tgreloid, key);
-
- if (OidIsValid(tgreloid))
+ PG_CATCH();
{
- /*
- * Input/output conversion for trigger tuples. Use the result
- * TypeInfo variable to store the tuple conversion info. We do this
- * over again on each call to cover the possibility that the
- * relation's tupdesc changed since the trigger was last called.
- * PLy_input_tuple_funcs and PLy_output_tuple_funcs are responsible
- * for not doing repetitive work.
- */
- TriggerData *tdata = (TriggerData *) fcinfo->context;
-
- Assert(CALLED_AS_TRIGGER(fcinfo));
- PLy_input_tuple_funcs(&(proc->result), tdata->tg_relation->rd_att);
- PLy_output_tuple_funcs(&(proc->result), tdata->tg_relation->rd_att);
+ /* Do not leave an uninitialised entry in the cache */
+ if (is_trigger)
+ hash_search(PLy_trigger_cache,
+ &fn_oid, HASH_REMOVE, NULL);
+ else
+ hash_search(PLy_procedure_cache,
+ &fn_oid, HASH_REMOVE, NULL);
+ PG_RE_THROW();
}
+ PG_END_TRY();
ReleaseSysCache(procTup);
- return proc;
+ return entry->proc;
}
+/*
+ * Create a new PLyProcedure structure
+ */
static PLyProcedure *
-PLy_procedure_create(HeapTuple procTup, Oid tgreloid, char *key)
+PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
{
char procName[NAMEDATALEN + 256];
Form_pg_proc procStruct;
@@ -1374,18 +1392,10 @@ PLy_procedure_create(HeapTuple procTup, Oid tgreloid, char *key)
rv;
procStruct = (Form_pg_proc) GETSTRUCT(procTup);
-
- if (OidIsValid(tgreloid))
- rv = snprintf(procName, sizeof(procName),
- "__plpython_procedure_%s_%u_trigger_%u",
- NameStr(procStruct->proname),
- HeapTupleGetOid(procTup),
- tgreloid);
- else
- rv = snprintf(procName, sizeof(procName),
- "__plpython_procedure_%s_%u",
- NameStr(procStruct->proname),
- HeapTupleGetOid(procTup));
+ rv = snprintf(procName, sizeof(procName),
+ "__plpython_procedure_%s_%u",
+ NameStr(procStruct->proname),
+ fn_oid);
if (rv >= sizeof(procName) || rv < 0)
elog(ERROR, "procedure name would overrun buffer");
@@ -1402,7 +1412,7 @@ PLy_procedure_create(HeapTuple procTup, Oid tgreloid, char *key)
PLy_typeinfo_init(&proc->args[i]);
proc->nargs = 0;
proc->code = proc->statics = NULL;
- proc->globals = proc->me = NULL;
+ proc->globals = NULL;
proc->is_setof = procStruct->proretset;
proc->setof = NULL;
proc->argnames = NULL;
@@ -1413,7 +1423,7 @@ PLy_procedure_create(HeapTuple procTup, Oid tgreloid, char *key)
* get information required for output conversion of the return value,
* but only if this isn't a trigger.
*/
- if (!OidIsValid(tgreloid))
+ if (!is_trigger)
{
HeapTuple rvTypeTup;
Form_pg_type rvTypeStruct;
@@ -1550,11 +1560,6 @@ PLy_procedure_create(HeapTuple procTup, Oid tgreloid, char *key)
pfree(procSource);
procSource = NULL;
-
- proc->me = PyCObject_FromVoidPtr(proc, NULL);
- if (!proc->me)
- PLy_elog(ERROR, "PyCObject_FromVoidPtr() failed");
- PyDict_SetItemString(PLy_procedure_cache, key, proc->me);
}
PG_CATCH();
{
@@ -1569,6 +1574,9 @@ PLy_procedure_create(HeapTuple procTup, Oid tgreloid, char *key)
return proc;
}
+/*
+ * Insert the procedure into the Python interpreter
+ */
static void
PLy_procedure_compile(PLyProcedure *proc, const char *src)
{
@@ -1591,7 +1599,7 @@ PLy_procedure_compile(PLyProcedure *proc, const char *src)
crv = PyRun_String(msrc, Py_file_input, proc->globals, NULL);
free(msrc);
- if (crv != NULL && (!PyErr_Occurred()))
+ if (crv != NULL)
{
int clen;
char call[NAMEDATALEN + 256];
@@ -1605,11 +1613,9 @@ PLy_procedure_compile(PLyProcedure *proc, const char *src)
if (clen < 0 || clen >= sizeof(call))
elog(ERROR, "string would overflow buffer");
proc->code = Py_CompileString(call, "<string>", Py_eval_input);
- if (proc->code != NULL && (!PyErr_Occurred()))
+ if (proc->code != NULL)
return;
}
- else
- Py_XDECREF(crv);
PLy_elog(ERROR, "could not compile PL/Python function \"%s\"", proc->proname);
}
@@ -1667,7 +1673,6 @@ PLy_procedure_delete(PLyProcedure *proc)
Py_XDECREF(proc->code);
Py_XDECREF(proc->statics);
Py_XDECREF(proc->globals);
- Py_XDECREF(proc->me);
if (proc->proname)
PLy_free(proc->proname);
if (proc->pyname)
@@ -1686,7 +1691,6 @@ PLy_procedure_delete(PLyProcedure *proc)
}
if (proc->argnames)
PLy_free(proc->argnames);
- PLy_free(proc);
}
/*
@@ -3232,6 +3236,7 @@ _PG_init(void)
/* Be sure we do initialization only once (should be redundant now) */
static bool inited = false;
const int **version_ptr;
+ HASHCTL hash_ctl;
if (inited)
return;
@@ -3263,9 +3268,20 @@ _PG_init(void)
PLy_init_plpy();
if (PyErr_Occurred())
PLy_elog(FATAL, "untrapped error in initialization");
- PLy_procedure_cache = PyDict_New();
- if (PLy_procedure_cache == NULL)
- PLy_elog(ERROR, "could not create procedure cache");
+
+ memset(&hash_ctl, 0, sizeof(hash_ctl));
+ hash_ctl.keysize = sizeof(Oid);
+ hash_ctl.entrysize = sizeof(PLyProcedureEntry);
+ hash_ctl.hash = oid_hash;
+ PLy_procedure_cache = hash_create("PL/Python procedures", 32, &hash_ctl,
+ HASH_ELEM | HASH_FUNCTION);
+
+ memset(&hash_ctl, 0, sizeof(hash_ctl));
+ hash_ctl.keysize = sizeof(Oid);
+ hash_ctl.entrysize = sizeof(PLyProcedureEntry);
+ hash_ctl.hash = oid_hash;
+ PLy_trigger_cache = hash_create("PL/Python triggers", 32, &hash_ctl,
+ HASH_ELEM | HASH_FUNCTION);
inited = true;
}
--
GitLab