Re: PL/Python initialization cleanup

From: Matheus Alcantara <matheusssilv97(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Python initialization cleanup
Date: 2026-01-12 18:07:21
Message-ID: 9fa042ab-bbcb-4818-95b3-cfac9737d775@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mihail Nikalayeu 2026-01-12 18:20:35 Re: Adding REPACK [concurrently]
Previous Message Artem Gavrilov 2026-01-12 18:04:19 Re: Timeline switching with partial WAL records can break replica recovery