On Tue, Apr 4, 2023 at 1:25 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Sorry for not looking at this sooner. I am okay with the regex
> changes proposed in v5-0001 through 0003, but I think you need to
> take another mopup pass there. Some specific complaints:
> * header comment for pg_regprefix has been falsified (s/malloc/palloc/)
Thanks. Fixed.
> * in spell.c, regex_affix_deletion_callback could be got rid of
Done in a separate patch. I wondered if regex_t should be included
directly as a member of that union inside AFFIX, but decided it should
keep using a pointer (just without the extra wrapper struct). A
direct member would make the AFFIX slightly larger, and it would
require us to assume that regex_t is movable which it probably
actually is in practice I guess but that isn't written down anywhere
and it seemed strange to rely on it.
> * check other callers of pg_regerror for now-useless CHECK_FOR_INTERRUPTS
I found three of these to remove (jsonpath_gram.y, varlena.c, test_regex.c).
> In general there's a lot of comments referring to regexes being malloc'd.
There is also some remaining direct use of malloc() in
regc_pg_locale.c because "we mustn't lose control on out-of-memory".
At that time (2012) there was no MCXT_NO_OOM (2015), so we could
presumably bring that cache into an observable MemoryContext now too.
I haven't written a patch for that, though, because it's not in the
way of my recovery conflict mission.
> I'm disinclined to change the ones inside the engine, because as far as
> it knows it is still using malloc, but maybe we should work harder on
> our own comments. In particular, it'd likely be useful to have something
> somewhere pointing out that pg_regfree is only needed when you can't
> get rid of the regex by context cleanup. Maybe write a short section
> about memory management in backend/regex/README?
I'll try to write something for the README tomorrow. Here's a new
version of the code changes.
> I've not really looked at 0004.
I'm hoping to get just the regex changes in ASAP, and then take a
little bit longer on the recovery conflict patch itself (v6-0005) on
the basis that it's bugfix work and not subject to the feature freeze.
