RE: PL/Python initialization cleanup

> On 12/01/26 09:24, Peter Eisentraut wrote:
> >> 0001, 0003 and 0004 looks good to me, just a small comment on 0002:
> >>
> >> -    /*
> >> -     * PyModule_AddObject does not add a refcount to the object, for
> >> some odd
> >> -     * reason; we must do that.
> >> -     */
> >> -    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.
> >> +     * permanent variable, so add another ref to account for that.
> >>        */
> >>       Py_INCREF(exc);
> >>
> >> The later code comment say "so add another ref to account for that",
> >> but you've removed the previous Py_INCREF() call. The returned object
> >> PyErr_NewException() already have a refcount increased for usage? If
> >> that's not the case I think that the "add another ref..." don't seems
> >> correct because IIUC we are increasing the ref count for the first
> >> time, so there is no "another" refcount being increased.
> >
> > The reference created by PyErr_NewException() is "stolen" by
> > PyModule_AddObject(), so we need to create another one for returning
> > the object from the function and storing it in the permanent variable.
> > I have updated the comment in this new patch version.  But I think the
> > actual code is correct.
> 
> Thanks, it looks more clear IMHO now. Agree that the code is correct.
> 
> 
> --
> Matheus Alcantara
> EDB: https://www.enterprisedb.com

Hi Peter,
I have applied and reviewed the v2 patch set. The cleanup of the initialization flow is a very good improvement and
makesthe logic much easier to follow.
 
In particular, the updated comments in PLy_create_exception regarding the reference counting logic are very helpful and
clarifythe previous ambiguity.
 
I have one minor nitpick in plpy_main.c regarding consistency. In _PG_init(), the import and dictionary insertion of
the"plpy" module is currently split into two checks with the same error message. To make it more consistent with how
the__main__ module is handled earlier in the same function, we could potentially streamline it like this:
 
    /*
     * Import plpy.
     */
    plpy_mod = PyImport_ImportModule("plpy");
    if (plpy_mod == NULL || PyDict_SetItemString(main_dict, "plpy", plpy_mod) < 0)
        PLy_elog(ERROR, "could not import \"plpy\" module");

This is just a small suggestion for style consistency; the existing logic in v2 is perfectly correct.

Regards,
Yuan Li(carol)