From aef325d073bee77a1b553dc38aaedb2bfb4d5eb1 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 30 Dec 2025 22:17:57 +0100 Subject: [PATCH v2 2/4] plpython: Clean up PyModule_AddObject() uses The comments "PyModule_AddObject does not add a refcount to the object, for some odd reason" seem distracting. Arguably, this behavior is expected, not odd. Also, the additional references created by the existing code do no seem necessary. But we should clean up the reference in the error case, as suggested by the Python documentation. Reviewed-by: Chao Li Reviewed-by: Matheus Alcantara Discussion: https://www.postgresql.org/message-id/f31333f1-fbb7-4098-b209-bf2d71fbd4f3%40eisentraut.org --- src/pl/plpython/plpy_plpymodule.c | 43 ++++++++++++++++--------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c index 66781291519..0d663ac5110 100644 --- a/src/pl/plpython/plpy_plpymodule.c +++ b/src/pl/plpython/plpy_plpymodule.c @@ -172,18 +172,6 @@ PLy_add_exceptions(PyObject *plpy) PyObject *excmod; HASHCTL hash_ctl; - excmod = PyModule_Create(&PLy_exc_module); - if (excmod == NULL) - PLy_elog(ERROR, "could not create the spiexceptions module"); - - /* - * PyModule_AddObject does not add a refcount to the object, for some odd - * reason; we must do that. - */ - Py_INCREF(excmod); - if (PyModule_AddObject(plpy, "spiexceptions", excmod) < 0) - PLy_elog(ERROR, "could not add the spiexceptions module"); - PLy_exc_error = PLy_create_exception("plpy.Error", NULL, NULL, "Error", plpy); PLy_exc_fatal = PLy_create_exception("plpy.Fatal", NULL, NULL, @@ -191,16 +179,28 @@ PLy_add_exceptions(PyObject *plpy) PLy_exc_spi_error = PLy_create_exception("plpy.SPIError", NULL, NULL, "SPIError", plpy); + excmod = PyModule_Create(&PLy_exc_module); + if (excmod == NULL) + PLy_elog(ERROR, "could not create the spiexceptions module"); + hash_ctl.keysize = sizeof(int); hash_ctl.entrysize = sizeof(PLyExceptionEntry); PLy_spi_exceptions = hash_create("PL/Python SPI exceptions", 256, &hash_ctl, HASH_ELEM | HASH_BLOBS); PLy_generate_spi_exceptions(excmod, PLy_exc_spi_error); + + if (PyModule_AddObject(plpy, "spiexceptions", excmod) < 0) + { + Py_XDECREF(excmod); + PLy_elog(ERROR, "could not add the spiexceptions module"); + } } /* * Create an exception object and add it to the module + * + * The created exception object is also returned. */ static PyObject * PLy_create_exception(char *name, PyObject *base, PyObject *dict, @@ -213,18 +213,19 @@ PLy_create_exception(char *name, PyObject *base, PyObject *dict, PLy_elog(ERROR, NULL); /* - * PyModule_AddObject does not add a refcount to the object, for some odd - * reason; we must do that. + * PyModule_AddObject() (below) steals the reference to exc, but we also + * want to return the value from this function, so add another ref to + * account for that. (The caller will store a pointer to the exception + * object in some permanent variable.) */ Py_INCREF(exc); - PyModule_AddObject(mod, modname, exc); - /* - * The caller will also store a pointer to the exception object in some - * permanent variable, so add another ref to account for that. This is - * probably excessively paranoid, but let's be sure. - */ - Py_INCREF(exc); + if (PyModule_AddObject(mod, modname, exc) < 0) + { + Py_XDECREF(exc); + PLy_elog(ERROR, "could not add exception %s", name); + } + return exc; } -- 2.52.0