Re: [PATCH] Allow complex data for GUC extra.

On 1/5/2026 2:28 PM, Robert Haas wrote:
> On Mon, Dec 29, 2025 at 9:24 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> 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