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

Bryan Green <dbryan(dot)green(at)gmail(dot)com> writes:
> On 11/18/2025 11:24 AM, Robert Haas wrote:
>> A whole different way of doing this would be to make the GUC machinery
>> responsible for spinning up and tearing down the contexts. Then, the
>> check hook could just be called with CurrentMemoryContext already set
>> to the new context, and the caller would know about it. Then, the
>> check hook doesn't need any special precautions to make sure the
>> context gets destroyed; instead, the GUC machinery takes care of that.

I like this in principle, but I don't think Bryan's implementation
sketch is right:

> 1. Create a temporary context (child of TopMemoryContext)

If the check_hook throws an error, you'll have leaked a long-lived
context. You must *not* make it a child of TopMemoryContext until
after successful assignment. I take Robert's point that we don't
know whether the GUC logic will be called in a context that is
short-lived or long-lived, so maybe making the context transiently
a child of CurrentMemoryContext isn't good enough ... but
TopMemoryContext is most definitely not good enough.

(Actually, these things should be children of GUCMemoryContext
not directly of TopMemoryContext. But that doesn't affect this
point, since those are equally long-lived.)

I'm really still dubious that this entire project is worthwhile.
I think it is basically building support for GUCs whose values
are unreasonably complicated, and would be better off if they
got redesigned. Also, right now might be a bad time to be
adding complexity to guc.c, in view of discussions such as [1].

regards, tom lane

[1] https://www.postgresql.org/message-id/flat/2ff46ac9-b46c-4210-8f0c-0f5365b36db9%40eisentraut.org