On 8/2/21, 1:37 PM, "Andres Freund" <andres@anarazel.de> wrote:
> On 2021-08-02 16:06:15 -0400, Robert Haas wrote:
>> I think this is a good idea. Anyone object?
>
> I'm not so sure it's a good idea. I've seen several shared_preload_library
> using extensions that adjust some GUCs (e.g. max_prepared_transactions)
> because they need some more resources internally - that's perhaps not a great
> idea, but there's also not an obviously better way.
Interesting. I hadn't heard of extensions adjusting GUCs in
_PG_init() before. I think the other way to handle that scenario is
to check the GUCs in _PG_init() and fail startup if necessary.
However, while it's probably good to avoid changing GUCs from what
users specified, failing startup isn't exactly user-friendly, either.
In any case, changing a GUC in _PG_init() seems quite risky. You're
effectively bypassing all of the usual checks.
> ISTM this would better be solved with making the hook or config logic for
> libraries a bit more elaborate (e.g. having a hook you can register for that's
> called once all libraries are loaded).
My worry is that this requires even more specialized knowledge to get
things right. If I need to do anything based on the value of a GUC,
I have to register another hook. In this new hook, we'd likely want
to somehow prevent modules from further adjusting GUCs. We'd also
need modules to check the GUCs they care about again in case another
module's _PG_init() adjusted it in an incompatible way. If you detect
a problem in this new hook, you probably need to fail startup.
Perhaps I am making a mountain out of a molehill and the modules that
are adjusting GUCs in _PG_init() are doing so in a generally safe way,
but IMO it ought to ordinarily be discouraged.
Nathan