Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4488142

Browse files
committed
Don't corrupt plpython's "TD" dictionary in a recursive trigger call.
If a plpython-language trigger caused another one to be invoked, the "TD" dictionary created for the inner one would overwrite the outer one's "TD" dictionary. This is more or less the same problem that 1d2fe56 fixed for ordinary functions in plpython, so fix it the same way, by saving and restoring "TD" during a recursive invocation. This fix makes an ABI-incompatible change in struct PLySavedArgs. I'm not too worried about that because it seems highly unlikely that any extension is messing with those structs. We could imagine doing something weird to preserve nominal ABI compatibility in the back branches, like keeping the saved TD object in an extra element of namedargs[]. However, that would only be very nominal compatibility: if anything *is* touching PLySavedArgs, it would likely do the wrong thing due to not knowing about the additional value. So I judge it not worth the ugliness to do something different there. (I also changed struct PLyProcedure, but its added field fits into formerly-padding space, so that should be safe.) Per bug #18456 from Jacques Combrink. This bug is very ancient, so back-patch to all supported branches. Discussion: https://postgr.es/m/3008982.1714853799@sss.pgh.pa.us
1 parent 01aeb43 commit 4488142

File tree

5 files changed

+81
-3
lines changed

5 files changed

+81
-3
lines changed

src/pl/plpython/expected/plpython_trigger.out

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,3 +618,30 @@ SELECT * FROM trigger_test_generated;
618618
---+---
619619
(0 rows)
620620

621+
-- recursive call of a trigger mustn't corrupt TD (bug #18456)
622+
CREATE TABLE recursive_trigger_test (a int, b int);
623+
CREATE FUNCTION recursive_trigger_func() RETURNS trigger
624+
LANGUAGE plpythonu
625+
AS $$
626+
if TD["event"] == "UPDATE":
627+
plpy.execute("INSERT INTO recursive_trigger_test VALUES (1, 2)")
628+
plpy.notice("TD[event] => " + str(TD["event"]) + ", expecting UPDATE");
629+
else:
630+
plpy.notice("TD[event] => " + str(TD["event"]) + ", expecting INSERT");
631+
return None
632+
$$;
633+
CREATE TRIGGER recursive_trigger_trig
634+
AFTER INSERT OR UPDATE ON recursive_trigger_test
635+
FOR EACH ROW EXECUTE PROCEDURE recursive_trigger_func();
636+
INSERT INTO recursive_trigger_test VALUES (0, 0);
637+
NOTICE: TD[event] => INSERT, expecting INSERT
638+
UPDATE recursive_trigger_test SET a = 11 WHERE b = 0;
639+
NOTICE: TD[event] => INSERT, expecting INSERT
640+
NOTICE: TD[event] => UPDATE, expecting UPDATE
641+
SELECT * FROM recursive_trigger_test;
642+
a | b
643+
----+---
644+
11 | 0
645+
1 | 2
646+
(2 rows)
647+

src/pl/plpython/plpy_exec.c

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,13 @@ PLy_exec_trigger(FunctionCallInfo fcinfo, PLyProcedure *proc)
339339
PLy_output_setup_tuple(&proc->result, rel_descr, proc);
340340
PLy_input_setup_tuple(&proc->result_in, rel_descr, proc);
341341

342+
/*
343+
* If the trigger is called recursively, we must push outer-level
344+
* arguments into the stack. This must be immediately before the PG_TRY
345+
* to ensure that the corresponding pop happens.
346+
*/
347+
PLy_global_args_push(proc);
348+
342349
PG_TRY();
343350
{
344351
int rc PG_USED_FOR_ASSERTS_ONLY;
@@ -405,13 +412,15 @@ PLy_exec_trigger(FunctionCallInfo fcinfo, PLyProcedure *proc)
405412
}
406413
PG_CATCH();
407414
{
415+
PLy_global_args_pop(proc);
408416
Py_XDECREF(plargs);
409417
Py_XDECREF(plrv);
410418

411419
PG_RE_THROW();
412420
}
413421
PG_END_TRY();
414422

423+
PLy_global_args_pop(proc);
415424
Py_DECREF(plargs);
416425
Py_DECREF(plrv);
417426

@@ -514,6 +523,13 @@ PLy_function_save_args(PLyProcedure *proc)
514523
result->args = PyDict_GetItemString(proc->globals, "args");
515524
Py_XINCREF(result->args);
516525

526+
/* If it's a trigger, also save "TD" */
527+
if (proc->is_trigger)
528+
{
529+
result->td = PyDict_GetItemString(proc->globals, "TD");
530+
Py_XINCREF(result->td);
531+
}
532+
517533
/* Fetch all the named arguments */
518534
if (proc->argnames)
519535
{
@@ -563,6 +579,13 @@ PLy_function_restore_args(PLyProcedure *proc, PLySavedArgs *savedargs)
563579
Py_DECREF(savedargs->args);
564580
}
565581

582+
/* Restore the "TD" object, too */
583+
if (savedargs->td)
584+
{
585+
PyDict_SetItemString(proc->globals, "TD", savedargs->td);
586+
Py_DECREF(savedargs->td);
587+
}
588+
566589
/* And free the PLySavedArgs struct */
567590
pfree(savedargs);
568591
}
@@ -581,8 +604,9 @@ PLy_function_drop_args(PLySavedArgs *savedargs)
581604
Py_XDECREF(savedargs->namedargs[i]);
582605
}
583606

584-
/* Drop ref to the "args" object, too */
607+
/* Drop refs to the "args" and "TD" objects, too */
585608
Py_XDECREF(savedargs->args);
609+
Py_XDECREF(savedargs->td);
586610

587611
/* And free the PLySavedArgs struct */
588612
pfree(savedargs);
@@ -591,9 +615,9 @@ PLy_function_drop_args(PLySavedArgs *savedargs)
591615
/*
592616
* Save away any existing arguments for the given procedure, so that we can
593617
* install new values for a recursive call. This should be invoked before
594-
* doing PLy_function_build_args().
618+
* doing PLy_function_build_args() or PLy_trigger_build_args().
595619
*
596-
* NB: caller must ensure that PLy_global_args_pop gets invoked once, and
620+
* NB: callers must ensure that PLy_global_args_pop gets invoked once, and
597621
* only once, per successful completion of PLy_global_args_push. Otherwise
598622
* we'll end up out-of-sync between the actual call stack and the contents
599623
* of proc->argstack.

src/pl/plpython/plpy_procedure.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
188188
proc->fn_readonly = (procStruct->provolatile != PROVOLATILE_VOLATILE);
189189
proc->is_setof = procStruct->proretset;
190190
proc->is_procedure = (procStruct->prokind == PROKIND_PROCEDURE);
191+
proc->is_trigger = is_trigger;
191192
proc->src = NULL;
192193
proc->argnames = NULL;
193194
proc->args = NULL;

src/pl/plpython/plpy_procedure.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ typedef struct PLySavedArgs
1616
{
1717
struct PLySavedArgs *next; /* linked-list pointer */
1818
PyObject *args; /* "args" element of globals dict */
19+
PyObject *td; /* "TD" element of globals dict, if trigger */
1920
int nargs; /* length of namedargs array */
2021
PyObject *namedargs[FLEXIBLE_ARRAY_MEMBER]; /* named args */
2122
} PLySavedArgs;
@@ -32,6 +33,7 @@ typedef struct PLyProcedure
3233
bool fn_readonly;
3334
bool is_setof; /* true, if function returns result set */
3435
bool is_procedure;
36+
bool is_trigger; /* called as trigger? */
3537
PLyObToDatum result; /* Function result output conversion info */
3638
PLyDatumToOb result_in; /* For converting input tuples in a trigger */
3739
char *src; /* textual procedure code, after mangling */

src/pl/plpython/sql/plpython_trigger.sql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,3 +467,27 @@ FOR EACH ROW EXECUTE PROCEDURE generated_test_func1();
467467
TRUNCATE trigger_test_generated;
468468
INSERT INTO trigger_test_generated (i) VALUES (1);
469469
SELECT * FROM trigger_test_generated;
470+
471+
472+
-- recursive call of a trigger mustn't corrupt TD (bug #18456)
473+
474+
CREATE TABLE recursive_trigger_test (a int, b int);
475+
476+
CREATE FUNCTION recursive_trigger_func() RETURNS trigger
477+
LANGUAGE plpythonu
478+
AS $$
479+
if TD["event"] == "UPDATE":
480+
plpy.execute("INSERT INTO recursive_trigger_test VALUES (1, 2)")
481+
plpy.notice("TD[event] => " + str(TD["event"]) + ", expecting UPDATE");
482+
else:
483+
plpy.notice("TD[event] => " + str(TD["event"]) + ", expecting INSERT");
484+
return None
485+
$$;
486+
487+
CREATE TRIGGER recursive_trigger_trig
488+
AFTER INSERT OR UPDATE ON recursive_trigger_test
489+
FOR EACH ROW EXECUTE PROCEDURE recursive_trigger_func();
490+
491+
INSERT INTO recursive_trigger_test VALUES (0, 0);
492+
UPDATE recursive_trigger_test SET a = 11 WHERE b = 0;
493+
SELECT * FROM recursive_trigger_test;

0 commit comments

Comments
 (0)