diff options
| -rw-r--r-- | src/pl/plpython/expected/plpython_trigger.out | 24 | ||||
| -rw-r--r-- | src/pl/plpython/plpy_main.c | 10 | ||||
| -rw-r--r-- | src/pl/plpython/plpy_procedure.c | 77 | ||||
| -rw-r--r-- | src/pl/plpython/plpy_procedure.h | 11 | ||||
| -rw-r--r-- | src/pl/plpython/sql/plpython_trigger.sql | 18 |
5 files changed, 99 insertions, 41 deletions
diff --git a/src/pl/plpython/expected/plpython_trigger.out b/src/pl/plpython/expected/plpython_trigger.out index 25060b09b78..80e478b3f20 100644 --- a/src/pl/plpython/expected/plpython_trigger.out +++ b/src/pl/plpython/expected/plpython_trigger.out @@ -610,3 +610,27 @@ SELECT * FROM composite_trigger_nested_test; ("(,t)","(1,f)",) (3 rows) +-- check that using a function as a trigger over two tables works correctly +CREATE FUNCTION trig1234() RETURNS trigger LANGUAGE plpythonu AS $$ + TD["new"]["data"] = '1234' + return 'MODIFY' +$$; +CREATE TABLE a(data text); +CREATE TABLE b(data int); -- different type conversion +CREATE TRIGGER a_t BEFORE INSERT ON a FOR EACH ROW EXECUTE PROCEDURE trig1234(); +CREATE TRIGGER b_t BEFORE INSERT ON b FOR EACH ROW EXECUTE PROCEDURE trig1234(); +INSERT INTO a DEFAULT VALUES; +SELECT * FROM a; + data +------ + 1234 +(1 row) + +DROP TABLE a; +INSERT INTO b DEFAULT VALUES; +SELECT * FROM b; + data +------ + 1234 +(1 row) + diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c index 8b61f1a972f..0dad8439565 100644 --- a/src/pl/plpython/plpy_main.c +++ b/src/pl/plpython/plpy_main.c @@ -14,6 +14,7 @@ #include "miscadmin.h" #include "utils/guc.h" #include "utils/memutils.h" +#include "utils/rel.h" #include "utils/syscache.h" #include "plpython.h" @@ -174,7 +175,8 @@ plpython_validator(PG_FUNCTION_ARGS) ReleaseSysCache(tuple); - PLy_procedure_get(funcoid, is_trigger); + /* We can't validate triggers against any particular table ... */ + PLy_procedure_get(funcoid, InvalidOid, is_trigger); PG_RETURN_VOID(); } @@ -215,20 +217,22 @@ plpython_call_handler(PG_FUNCTION_ARGS) PG_TRY(); { + Oid funcoid = fcinfo->flinfo->fn_oid; PLyProcedure *proc; if (CALLED_AS_TRIGGER(fcinfo)) { + Relation tgrel = ((TriggerData *) fcinfo->context)->tg_relation; HeapTuple trv; - proc = PLy_procedure_get(fcinfo->flinfo->fn_oid, true); + proc = PLy_procedure_get(funcoid, RelationGetRelid(tgrel), true); exec_ctx->curr_proc = proc; trv = PLy_exec_trigger(fcinfo, proc); retval = PointerGetDatum(trv); } else { - proc = PLy_procedure_get(fcinfo->flinfo->fn_oid, false); + proc = PLy_procedure_get(funcoid, InvalidOid, false); exec_ctx->curr_proc = proc; retval = PLy_exec_function(fcinfo, proc); } diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c index a28cb9a41f3..5007e7770ff 100644 --- a/src/pl/plpython/plpy_procedure.c +++ b/src/pl/plpython/plpy_procedure.c @@ -24,7 +24,6 @@ static HTAB *PLy_procedure_cache = NULL; -static HTAB *PLy_trigger_cache = NULL; static PLyProcedure *PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger); static bool PLy_procedure_argument_valid(PLyTypeInfo *arg); @@ -38,18 +37,11 @@ init_procedure_caches(void) HASHCTL hash_ctl; memset(&hash_ctl, 0, sizeof(hash_ctl)); - hash_ctl.keysize = sizeof(Oid); + hash_ctl.keysize = sizeof(PLyProcedureKey); hash_ctl.entrysize = sizeof(PLyProcedureEntry); - hash_ctl.hash = oid_hash; + hash_ctl.hash = tag_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); } /* @@ -69,61 +61,74 @@ PLy_procedure_name(PLyProcedure *proc) /* * PLy_procedure_get: returns a cached PLyProcedure, or creates, stores and - * returns a new PLyProcedure. fcinfo is the call info, tgreloid is the - * relation OID when calling a trigger, or InvalidOid (zero) for ordinary - * function calls. + * returns a new PLyProcedure. + * + * fn_oid is the OID of the function requested + * fn_rel is InvalidOid or the relation this function triggers on + * is_trigger denotes whether the function is a trigger function + * + * The reason that both fn_rel and is_trigger need to be passed is that when + * trigger functions get validated we don't know which relation(s) they'll + * be used with, so no sensible fn_rel can be passed. */ PLyProcedure * -PLy_procedure_get(Oid fn_oid, bool is_trigger) +PLy_procedure_get(Oid fn_oid, Oid fn_rel, bool is_trigger) { + bool use_cache = !(is_trigger && fn_rel == InvalidOid); HeapTuple procTup; - PLyProcedureEntry *volatile entry; - bool found; + PLyProcedureKey key; + PLyProcedureEntry *volatile entry = NULL; + PLyProcedure *volatile proc = NULL; + bool found = false; procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fn_oid)); if (!HeapTupleIsValid(procTup)) elog(ERROR, "cache lookup failed for function %u", fn_oid); - /* 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); + /* + * Look for the function in the cache, unless we don't have the necessary + * information (e.g. during validation). In that case we just don't cache + * anything. + */ + if (use_cache) + { + key.fn_oid = fn_oid; + key.fn_rel = fn_rel; + entry = hash_search(PLy_procedure_cache, &key, HASH_ENTER, &found); + proc = entry->proc; + } PG_TRY(); { if (!found) { - /* Haven't found it, create a new cache entry */ - entry->proc = PLy_procedure_create(procTup, fn_oid, is_trigger); + /* Haven't found it, create a new procedure */ + proc = PLy_procedure_create(procTup, fn_oid, is_trigger); + if (use_cache) + entry->proc = proc; } - else if (!PLy_procedure_valid(entry->proc, procTup)) + else if (!PLy_procedure_valid(proc, procTup)) { /* 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); + PLy_procedure_delete(proc); + PLy_free(proc); + proc = PLy_procedure_create(procTup, fn_oid, is_trigger); + entry->proc = proc; } /* Found it and it's valid, it's fine to use it */ } PG_CATCH(); { /* 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); + if (use_cache) + hash_search(PLy_procedure_cache, &key, HASH_REMOVE, NULL); PG_RE_THROW(); } PG_END_TRY(); ReleaseSysCache(procTup); - return entry->proc; + return proc; } /* diff --git a/src/pl/plpython/plpy_procedure.h b/src/pl/plpython/plpy_procedure.h index 40a0314cdfb..f1c8510dafc 100644 --- a/src/pl/plpython/plpy_procedure.h +++ b/src/pl/plpython/plpy_procedure.h @@ -32,16 +32,23 @@ typedef struct PLyProcedure PyObject *globals; /* data saved across calls, global scope */ } PLyProcedure; +/* the procedure cache key */ +typedef struct PLyProcedureKey +{ + Oid fn_oid; /* function OID */ + Oid fn_rel; /* triggered-on relation or InvalidOid */ +} PLyProcedureKey; + /* the procedure cache entry */ typedef struct PLyProcedureEntry { - Oid fn_oid; /* hash key */ + PLyProcedureKey key; /* hash key */ PLyProcedure *proc; } PLyProcedureEntry; /* PLyProcedure manipulation */ extern char *PLy_procedure_name(PLyProcedure *proc); -extern PLyProcedure *PLy_procedure_get(Oid fn_oid, bool is_trigger); +extern PLyProcedure *PLy_procedure_get(Oid fn_oid, Oid fn_rel, bool is_trigger); extern void PLy_procedure_compile(PLyProcedure *proc, const char *src); extern void PLy_procedure_delete(PLyProcedure *proc); diff --git a/src/pl/plpython/sql/plpython_trigger.sql b/src/pl/plpython/sql/plpython_trigger.sql index 9727f44f8b4..a054fe729bc 100644 --- a/src/pl/plpython/sql/plpython_trigger.sql +++ b/src/pl/plpython/sql/plpython_trigger.sql @@ -388,3 +388,21 @@ INSERT INTO composite_trigger_nested_test VALUES (NULL); INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(1, 'f'), NULL, 3)); INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(NULL, 't'), ROW(1, 'f'), NULL)); SELECT * FROM composite_trigger_nested_test; + +-- check that using a function as a trigger over two tables works correctly +CREATE FUNCTION trig1234() RETURNS trigger LANGUAGE plpythonu AS $$ + TD["new"]["data"] = '1234' + return 'MODIFY' +$$; + +CREATE TABLE a(data text); +CREATE TABLE b(data int); -- different type conversion + +CREATE TRIGGER a_t BEFORE INSERT ON a FOR EACH ROW EXECUTE PROCEDURE trig1234(); +CREATE TRIGGER b_t BEFORE INSERT ON b FOR EACH ROW EXECUTE PROCEDURE trig1234(); + +INSERT INTO a DEFAULT VALUES; +SELECT * FROM a; +DROP TABLE a; +INSERT INTO b DEFAULT VALUES; +SELECT * FROM b; |