| From: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
|---|---|
| To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: PL/Python initialization cleanup |
| Date: | 2026-01-14 21:03:07 |
| Message-ID: | CALdSSPitnbaedCiafJ-sUp6BsCPXqKqUL13_rQ+1w8ZKfnuL6g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, 12 Jan 2026 at 17:25, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 01.01.26 00:34, Chao Li wrote:
> >
> >
> >> On Dec 31, 2025, at 16:47, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> >>
> >> As I was working through steps to make PL/Python more thread-safe, I noticed that the initialization code of PL/Python is pretty messy. I think some of this has grown while both Python 2 and 3 were supported, because they required different initialization steps, and we had some defenses against accidentally running both at the same time. But that is over, and right now a lot of this doesn't make sense anymore. For example, the function PLy_init_interp() said "Initialize the Python interpreter ..." but it didn't actually do this, and PLy_init_plpy() said "initialize plpy module" but it didn't do that either (or at least they used the term "initialize" in non-standard ways).
> >>
> >> Here are some patches to clean this up. After this change, all the global initialization is called directly from _PG_init(), and the plpy module initialization is all called from its registered initialization function PyInit_plpy(). (For the thread-safety job, the plpy module initialization will need to be rewritten using a different API. That's why I'm keen to have it clearly separated.) I also tried to add more comments and make existing comments more precise. There was also some apparently obsolete or redundant code that could be deleted.
> >>
> >> Surely, all of this will need some more rounds of careful scrutiny, but I think the overall code arrangement is correct and an improvement.
> >> <v1-0001-plpython-Remove-commented-out-code.patch><v1-0002-plpython-Clean-up-PyModule_AddObject-uses.patch><v1-0003-plpython-Remove-duplicate-PyModule_Create.patch><v1-0004-plpython-Streamline-initialization.patch>
> >
> > I just did an eyeball review. Overall looks good to me. The cleanup, as explained in the patch email, makes sense to me. Only a nit comment on 0002:
> >
> > 1 - 0002
> > ```
> > + if (PyModule_AddObject(mod, modname, exc) < 0)
> > + {
> > + Py_XDECREF(exc);
> > + PLy_elog(ERROR, "could not add exceptions %s", name);
> > + }
> > ```
> >
> > Plural “exceptions” is a little confusing. What about “could not add exception object”?
>
> Thanks, I have fixed this in the v2 patch (sent in a separate message).
>
hi!
0001, 0002, 0003, are ready, LGTM.
For 0004, do we need main_dict at all? it is only used inside _PG_init
and then its value assigned to PLy_interp_globals...
--
Best regards,
Kirill Reshke
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David Rowley | 2026-01-14 21:10:55 | Re: Patch: dumping tables data in multiple chunks in pg_dump |
| Previous Message | Matheus Alcantara | 2026-01-14 21:01:58 | Re: support ALTER COLUMN SET EXPRESSION over virtual generated column with check constraint |