On Mon, Nov 17, 2025 at 4:17 PM Bryan Green <dbryan.green@gmail.com> wrote:
> The solution uses a wrapper struct (GucContextExtra) containing both the
> MemoryContext and data pointers. Check hooks:
> 1. Create a context under CurrentMemoryContext (for error safety)
> 2. Allocate their data structures within it
> 3. Allocate the wrapper itself within the same context
> 4. On success, re-parent the context to TopMemoryContext
> 5. Return the wrapper as extra
An alternative design would be to make the check hook simply return a
chunk palloc'd from the new context, and the GUC machinery would use
GetMemoryChunkContext() to recover the context pointer and then
MemoryContextDelete that context. I'm not sure if that's better or
worse.
I think one of the big usability questions around this is how the
check hook is supposed to avoid leaking if it errors out. The approach
you've taken is to have the check hook create the context under
CurrentMemoryContext and then reparent it just before returning, which
may be fine, but is worth discussing. I'm not 100% sure that it's
actually good enough for every case: is there no situation where a
check hook can be called without a CurrentMemoryContext, or with a
very long-lived memory context like TopMemoryContext set to current?
Even if there's technically a leak here, maybe we don't care: it might
be limited enough not to matter.
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.
Here again, I'm not sure if this is better or worse than what you
have.
Thanks for working on this.
--
Robert Haas
EDB: http://www.enterprisedb.com
