On 1/5/2026 2:28 PM, Robert Haas wrote: > On Mon, Dec 29, 2025 at 9:24 PM Tom Lane <[email protected]> wrote: >> The key reason I'm allergic to this is that throwing elog(ERROR) in >> the postmaster process will take down the postmaster. So we really >> do not want code that will execute during SIGHUP configuration >> reloads to be doing that. I grant that there will probably always >> be edge cases where that happens, but I'm not okay with building >> such a hazard into the GUC APIs. > > This is a tough one. In most of PostgreSQL, you can just throw an > error when something goes wrong and let error cleanup handle it. So, > we have a lot of code that works like that: it just throws an error > and assumes that the right things happen afterwards. If you're in a > context where you can't just throw an error, you have to avoid not > only throwing errors directly but also calling any code that might do > so on your behalf -- which is very awkward, because it means there's a > huge amount of backend code that you can't think about calling. In a > case like this, for example, let's say you're calling into a flex + > bison lexer/parser. Very often we code such things to "just throw an > error," and you couldn't do that here. You'd have to arrange to return > errors up the call stack, which is no doubt possible, but it's not > very pleasant. > > I feel like this "you can't throw an error thing" is a worse > limitation than "it has to be a single palloc'd chunk". I wonder if we > could design some infrastructure to get out from under that > requirement. > > I guess that's properly a separate patch/project, but just think about > how annoying this is going to be to use. To go back to the example of > a flex + bison lexer/parser, say that when we are called from a GUC's > check hook, if we adopted what Bryan proposes in his reply to this > email, we would need to use guc_palloc() for all memory allocations. > But if we're parsing a value that came from someplace other than a > GUC, we would need to use regular palloc(). If we rejected Bryan's > guc_palloc() idea, it's better: we can use Tom's proposal of > palloc_extended(..., MCXT_ALLOC_NO_OOM) in both cases. It's just going > to be annoying to do that everyplace that we allocate memory. > > I think it's probably still better than not having the facility > proposed here at all, but it certainly seems like it makes code reuse > noticeably harder. >
Robert, Tom, Thanks for the continued feedback. I think I understand your point about palloc_extended(...) being better for code reuse than guc_palloc(...). The guc_palloc(...) function is semantically tied to GUC contexts by both naming and behavior (logging but returning null rather than throwing). This was modeled after guc_malloc, but I can see how the use case of shared code would feel more natural using palloc_extended(...) as opposed to guc_palloc-- after all, why would you call guc_palloc in a parser when in a regular backend context as opposed to a GUC hook context. I will make the changes to the patch to use palloc_extended(...). Thanks, -- Bryan Green EDB: https://www.enterprisedb.com
