8000 Fix plpython's handling of functions used as triggers on multiple tab… · danielcode/postgres@308711a · GitHub
[go: up one dir, main page]
Skip to content < 8000 script type="application/json" data-target="react-partial.embeddedData">{"props":{"docsUrl":"https://docs.github.com/get-started/accessibility/keyboard-shortcuts"}}

Commit 308711a

Browse files
tglsfdctglsfdc
committed
Fix plpython's handling of functions used as triggers on multiple tables.
plpython tried to use a single cache entry for a trigger function, but it needs a separate cache entry for each table the trigger is applied to, because there is table-dependent data in there. This was done correctly before 9.1, but commit 46211da broke it by simplifying the lookup key from "function OID and triggered table OID" to "function OID and is-trigger boolean". Go back to using both OIDs as the lookup key. Per bug report from Sandro Santilli. Andres Freund
1 parent 6d5c62a commit 308711a

File tree

5 files changed

+99
-41
lines changed

5 files changed

+99
-41
lines changed

‎src/pl/plpython/expected/plpython_trigger.out

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,3 +610,27 @@ SELECT * FROM composite_trigger_nested_test;
610610
("(,t)","(1,f)",)
611611
(3 rows)
612612

613+
-- check that using a function as a trigger over two tables works correctly
614+
CREATE FUNCTION trig1234() RETURNS trigger LANGUAGE plpythonu AS $$
615+
TD["new"]["data"] = '1234'
616+
return 'MODIFY'
617+
$$;
618+
CREATE TABLE a(data text);
619+
CREATE TABLE b(data int); -- different type conversion
620+
CREATE TRIGGER a_t BEFORE INSERT ON a FOR EACH ROW EXECUTE PROCEDURE trig1234();
621+
CREATE TRIGGER b_t BEFORE INSERT ON b FOR EACH ROW EXECUTE PROCEDURE trig1234();
622+
INSERT INTO a DEFAULT VALUES;
623+
SELECT * FROM a;
624+
data
625+
------
626+
1234
627+
(1 row)
628+
629+
DROP TABLE a;
630+
INSERT INTO b DEFAULT VALUES;
631+
SELECT * FROM b;
632+
data
633+
------
634+
1234
635+
(1 row)
636+

‎src/pl/plpython/plpy_main.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "miscadmin.h"
1414
#include "utils/guc.h"
1515
#include "utils/memutils.h"
16+
#include "utils/rel.h"
1617
#include "utils/syscache.h"
1718

1819
#include "plpython.h"
@@ -173,7 +174,8 @@ plpython_validator(PG_FUNCTION_ARGS)
173174

174175
ReleaseSysCache(tuple);
175176

176-
PLy_procedure_get(funcoid, is_trigger);
177+
/* We can't validate triggers against any particular table ... */
178+
PLy_procedure_get(funcoid, InvalidOid, is_trigger);
177179

178180
PG_RETURN_VOID();
179181
}
@@ -214,20 +216,22 @@ plpython_call_handler(PG_FUNCTION_ARGS)
214216

215217
PG_TRY();
216218
{
219+
Oid funcoid = fcinfo->flinfo->fn_oid;
217220
PLyProcedure *proc;
218221

219222
if (CALLED_AS_TRIGGER(fcinfo))
220223
{
224+
Relation tgrel = ((TriggerData *) fcinfo->context)->tg_relation;
221225
HeapTuple trv;
222226

223-
proc = PLy_procedure_get(fcinfo->flinfo->fn_oid, true);
227+
proc = PLy_procedure_get(funcoid, RelationGetRelid(tgrel), true);
224228
exec_ctx->curr_proc = proc;
225229
trv = PLy_exec_trigger(fcinfo, proc);
226230
retval = PointerGetDatum(trv);
227231
}
228232
else
229233
{
230-
proc = PLy_procedure_get(fcinfo->flinfo->fn_oid, false);
234+
proc = PLy_procedure_get(funcoid, InvalidOid, false);
231235
exec_ctx->curr_proc = proc;
232236
retval = PLy_exec_function(fcinfo, proc);
233237
}

‎src/pl/plpython/plpy_procedure.c

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323

2424

2525
static HTAB *PLy_procedure_cache = NULL;
26-
static HTAB *PLy_trigger_cache = NULL;
2726

2827
static PLyProcedure *PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger);
2928
static bool PLy_procedure_argument_valid(PLyTypeInfo *arg);
@@ -37,18 +36,11 @@ init_procedure_caches(void)
3736
HASHCTL hash_ctl;
3837

3938
memset(&hash_ctl, 0, sizeof(hash_ctl));
40-
hash_ctl.keysize = sizeof(Oid);
39+
hash_ctl.keysize = sizeof(PLyProcedureKey);
4140
hash_ctl.entrysize = sizeof(PLyProcedureEntry);
42-
hash_ctl.hash = oid_hash;
41+
hash_ctl.hash = tag_hash;
4342
PLy_procedure_cache = hash_create("PL/Python procedures", 32, &hash_ctl,
4443
HASH_ELEM | HASH_FUNCTION);
45-
46-
memset(&hash_ctl, 0, sizeof(hash_ctl));
47-
hash_ctl.keysize = sizeof(Oid);
48-
hash_ctl.entrysize = sizeof(PLyProcedureEntry);
49-
hash_ctl.hash = oid_hash;
50-
PLy_trigger_cache = hash_create("PL/Python triggers", 32, &hash_ctl,
51-
HASH_ELEM | HASH_FUNCTION);
5244
}
5345

5446
/*
@@ -68,61 +60,74 @@ PLy_procedure_name(PLyProcedure *proc)
6860

6961
/*
7062
* PLy_procedure_get: returns a cached PLyProcedure, or creates, stores and
71-
* returns a new PLyProcedure. fcinfo is the call info, tgreloid is the
72-
* relation OID when calling a trigger, or InvalidOid (zero) for ordinary
73-
* function calls.
63+
* returns a new PLyProcedure.
64+
*
65+
* fn_oid is the OID of the function requested
66+
* fn_rel is InvalidOid or the relation this function triggers on
67+
* is_trigger denotes whether the function is a trigger function
68+
*
69+
* The reason that both fn_rel and is_trigger need to be passed is that when
70+
* trigger functions get validated we don't know which relation(s) they'll
71+
* be used with, so no sensible fn_rel can be passed.
7472
*/
7573
PLyProcedure *
76-
PLy_procedure_get(Oid fn_oid, bool is_trigger)
74+
PLy_procedure_get(Oid fn_oid, Oid fn_rel, bool is_trigger)
7775
{
76+
bool use_cache = !(is_trigger && fn_rel == InvalidOid);
7877
HeapTuple procTup;
79-
PLyProcedureEntry *volatile entry;
80-
bool found;
78+
PLyProcedureKey key;
79+
PLyProcedureEntry *volatile entry = NULL;
80+
PLyProcedure *volatile proc = NULL;
81+
bool found = false;
8182

8283
procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(fn_oid));
8384
if (!HeapTupleIsValid(procTup))
8485
elog(ERROR, "cache lookup failed for function %u", fn_oid);
8586

86-
/* Look for the function in the corresponding cache */
87-
if (is_trigger)
88-
entry = hash_search(PLy_trigger_cache,
89-
&fn_oid, HASH_ENTER, &found);
90-
else
91-
entry = hash_search(PLy_procedure_cache,
92-
&fn_oid, HASH_ENTER, &found);
87+
/*
88+
* Look for the function in the cache, unless we don't have the necessary
89+
* information (e.g. during validation). In that case we just don't cache
90+
* anything.
91+
*/
92+
if (use_cache)
93+
{
94+
key.fn_oid = fn_oid;
95+
key.fn_rel = fn_rel;
96+
entry = hash_search(PLy_procedure_cache, &key, HASH_ENTER, &found);
97+
proc = entry->proc;
98+
}
9399

94100
PG_TRY();
95101
{
96102
if (!found)
97103
{
98-
/* Haven't found it, create a new cache entry */
99-
entry->proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
104+
/* Haven't found it, create a new procedure */
105+
proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
106+
if (use_cache)
107+
entry->proc = proc;
100108
}
101-
else if (!PLy_procedure_valid(entry->proc, procTup))
109+
else if (!PLy_procedure_valid(proc, procTup))
102110
{
103111
/* Found it, but it's invalid, free and reuse the cache entry */
104-
PLy_procedure_delete(entry->proc);
105-
PLy_free(entry->proc);
106-
entry->proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
112+
PLy_procedure_delete(proc);
113+
PLy_free(proc);
114+
proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
115+
entry->proc = proc;
107116
}
108117
/* Found it and it's valid, it's fine to use it */
109118
}
110119
PG_CATCH();
111120
{
112121
/* Do not leave an uninitialised entry in the cache */
113-
if (is_trigger)
114-
hash_search(PLy_trigger_cache,
115-
&fn_oid, HASH_REMOVE, NULL);
116-
else
117-
hash_search(PLy_procedure_cache,
118-
&fn_oid, HASH_REMOVE, NULL);
122+
if (use_cache)
123+
hash_search(PLy_procedure_cache, &key, HASH_REMOVE, NULL);
119124
PG_RE_THROW();
120125
}
121126
PG_END_TRY();
122127

123128
ReleaseSysCache(procTup);
124129

125-
return entry->proc;
130+
return proc;
126131
}
127132

128133
/*

‎src/pl/plpython/plpy_procedure.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,23 @@ typedef struct PLyProcedure
3232
PyObject *globals; /* data saved across calls, global scope */
3333
} PLyProcedure;
3434

35+
/* the procedure cache key */
36+
typedef struct PLyProcedureKey
37+
{
38+
Oid fn_oid; /* function OID */
39+
Oid fn_rel; /* triggered-on relation or InvalidOid */
40+
} PLyProcedureKey;
41+
3542
/* the procedure cache entry */
3643
typedef struct PLyProcedureEntry
3744
{
38-
Oid fn_oid; /* hash key */
45+
PLyProcedureKey key; /* hash key */
3946
PLyProcedure *proc;
4047
} PLyProcedureEntry;
4148

4249
/* PLyProcedure manipulation */
4350
extern char *PLy_procedure_name(PLyProcedure *proc);
44-
extern PLyProcedure *PLy_procedure_get(Oid fn_oid, bool is_trigger);
51+
extern PLyProcedure *PLy_procedure_get(Oid fn_oid, Oid fn_rel, bool is_trigger);
4552
extern void PLy_procedure_compile(PLyProcedure *proc, const char *src);
4653
extern void PLy_procedure_delete(PLyProcedure *proc);
4754

‎src/pl/plpython/sql/plpython_trigger.sql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,3 +388,21 @@ INSERT INTO composite_trigger_nested_test VALUES (NULL);
388388
INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(1, 'f'), NULL, 3));
389389
INSERT INTO composite_trigger_nested_test VALUES (ROW(ROW(NULL, 't'), ROW(1, 'f'), NULL));
390390
SELECT * FROM composite_trigger_nested_test;
391+
392+
-- check that using a function as a trigger over two tables works correctly
393+
CREATE FUNCTION trig1234() RETURNS trigger LANGUAGE plpythonu AS $$
394+
TD["new"]["data"] = '1234'
395+
return 'MODIFY'
396+
$$;
397+
398+
CREATE TABLE a(data text);
399+
CREATE TABLE b(data int); -- different type conversion
400+
401+
CREATE TRIGGER a_t BEFORE INSERT ON a FOR EACH ROW EXECUTE PROCEDURE trig1234();
402+
CREATE TRIGGER b_t BEFORE INSERT ON b FOR EACH ROW EXECUTE PROCEDURE trig1234();
403+
404+
INSERT INTO a DEFAULT VALUES;
405+
SELECT * FROM a;
406+
DROP TABLE a;
407+
INSERT INTO b DEFAULT VALUES;
408+
SELECT * FROM b;

0 commit comments

Comments
 (0)
0