From ef8b09ac5650fff344440bdec1280cc81612225b Mon Sep 17 00:00:00 2001 From: Xing Guo Date: Tue, 14 Jul 2026 16:12:00 +0800 Subject: [PATCH v2] plpython: pair SPI_connect and SPI_finish in the same function PL/Python's call_handler, inline_handler call SPI_connect_ext() at its top, but SPI_finish() is called inside execution functions PLy_exec_function, PLy_exec_trigger, PLy_exec_event_trigger in plpy_exec.c. It's easy to miss SPI_finish when adding a new execution path for PL/Python. SPI_finish() are moved into the two handlers and paired with SPI_connect() in the same function. This matches code patterns in plpgsql and plperl. Signed-off-by: Xing Guo --- src/pl/plpython/plpy_exec.c | 19 ++++++++----------- src/pl/plpython/plpy_main.c | 25 +++++++++---------------- 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c index de7f64113d6..d3b470c48e0 100644 --- a/src/pl/plpython/plpy_exec.c +++ b/src/pl/plpython/plpy_exec.c @@ -172,13 +172,13 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedureCache *pcache) } /* - * Disconnect from SPI manager and then create the return values datum + * Switch out of SPI's memory context and then create the return values + * datum * (if the input function does a palloc for it this must not be - * allocated in the SPI memory context because SPI_finish would free - * it). + * allocated in the SPI memory context because the upper caller would + * call SPI_finish to free it). */ - if (SPI_finish() != SPI_OK_FINISH) - elog(ERROR, "SPI_finish failed"); + MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt); plerrcontext.callback = plpython_return_error_callback; plerrcontext.previous = error_context_stack; @@ -439,10 +439,10 @@ PLy_exec_trigger(FunctionCallInfo fcinfo, PLyProcedure *proc) Assert(plrv != NULL); /* - * Disconnect from SPI manager + * Switch out of SPI's memory context so that trigger result + * processing doesn't allocate inside SPI's procedure context. */ - if (SPI_finish() != SPI_OK_FINISH) - elog(ERROR, "SPI_finish failed"); + MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt); /* * return of None means we're happy with the tuple @@ -527,9 +527,6 @@ PLy_exec_event_trigger(FunctionCallInfo fcinfo, PLyProcedure *proc) Py_DECREF(plttag); PLy_procedure_call(proc, "TD", pltdata); - - if (SPI_finish() != SPI_OK_FINISH) - elog(ERROR, "SPI_finish() failed"); } PG_FINALLY(); { diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c index df08d3fbd1b..5b2ecb52425 100644 --- a/src/pl/plpython/plpy_main.c +++ b/src/pl/plpython/plpy_main.c @@ -188,7 +188,6 @@ plpython3_call_handler(PG_FUNCTION_ARGS) IsA(fcinfo->context, CallContext) && !castNode(CallContext, fcinfo->context)->atomic; - /* Note: SPI_finish() happens in plpy_exec.c, which is dubious design */ SPI_connect_ext(nonatomic ? SPI_OPT_NONATOMIC : 0); /* @@ -236,18 +235,16 @@ plpython3_call_handler(PG_FUNCTION_ARGS) else retval = PLy_exec_function(fcinfo, pcache); } - PG_CATCH(); + PG_FINALLY(); { - /* Destroy the execution context */ + /* Destroy the execution context on both success and error paths */ PLy_pop_execution_context(); PyErr_Clear(); - - PG_RE_THROW(); } PG_END_TRY(); - /* Destroy the execution context */ - PLy_pop_execution_context(); + if (SPI_finish() != SPI_OK_FINISH) + elog(ERROR, "SPI_finish failed"); return retval; } @@ -263,7 +260,6 @@ plpython3_inline_handler(PG_FUNCTION_ARGS) PLyExecutionContext *exec_ctx; ErrorContextCallback plerrcontext; - /* Note: SPI_finish() happens in plpy_exec.c, which is dubious design */ SPI_connect_ext(codeblock->atomic ? 0 : SPI_OPT_NONATOMIC); MemSet(fcinfo, 0, SizeForFunctionCallInfo(0)); @@ -313,20 +309,17 @@ plpython3_inline_handler(PG_FUNCTION_ARGS) exec_ctx->curr_proc = &proc; PLy_exec_function(fake_fcinfo, &pcache); } - PG_CATCH(); + PG_FINALLY(); { + /* Destroy the execution context on both success and error paths */ PLy_pop_execution_context(); - PLy_procedure_delete(&proc); PyErr_Clear(); - PG_RE_THROW(); + PLy_procedure_delete(&proc); } PG_END_TRY(); - /* Destroy the execution context */ - PLy_pop_execution_context(); - - /* Now clean up the transient procedure we made */ - PLy_procedure_delete(&proc); + if (SPI_finish() != SPI_OK_FINISH) + elog(ERROR, "SPI_finish failed"); PG_RETURN_VOID(); } -- 2.55.0