Re: warn if GUC set to an invalid shared library

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Cary Huang <cary(dot)huang(at)highgo(dot)ca>, Maciek Sakrejda <m(dot)sakrejda(at)gmail(dot)com>
Subject: Re: warn if GUC set to an invalid shared library
Date: 2024-05-24 15:48:34
Message-ID: ZlC20rkDhfwt4cYe@pryzbyj2023
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 24, 2024 at 09:26:54AM -0400, Robert Haas wrote:
> On Thu, Jul 6, 2023 at 4:15 PM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:

> But then I realized, reading another email, that Justin already knew
> that the behavior would change, or at least I'm 90% certain that he
> knows that.

You give me too much credit..

> On the behavior change itself, it seems to me that there's a big
> difference between shared_preload_libraries=bogus and work_mem=bogus.
..
> So if changing PGC_S_FILE to
> PGC_S_TEST in AlterSystemSetConfigFile is going to have the effect of
> allowing garbage values into postgresql.auto.conf that would currently
> get blocked, I think that's a bad plan and we shouldn't do it.

Right - this is something I'd failed to realize. We can't change it in
the naive way because it allows bogus values, and not just missing
libraries. Specifically, for GUCs with assign hooks conditional on
PGC_TEST.

We don't want to change the behavior to allow this to succeed -- it
would allow leaving the server in a state that it fails to start (rather
than helping to avoid doing so, as intended by this thread).

regression=# ALTER SYSTEM SET default_table_access_method=abc;
NOTICE: table access method "abc" does not exist
ALTER SYSTEM

Maybe there should be a comment explaning why PGC_FILE is used, and
maybe there should be a TAP test for the behavior, but they're pretty
unrelated to this thread. So, I've dropped the 001 patch.

--
Justin

Attachment Content-Type Size
0001-warn-when-setting-GUC-to-a-nonextant-library.patch text/x-diff 9.6 KB
0002-errcontext-if-server-fails-to-start-due-to-library-G.patch text/x-diff 4.5 KB
0003-show-the-GUC-source-in-errcontext.patch text/x-diff 3.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-05-24 15:59:10 Re: DROP OWNED BY fails to clean out pg_init_privs grants
Previous Message Jacob Champion 2024-05-24 15:43:01 Re: pg_parse_json() should not leak token copies on failure