Re: warn if GUC set to an invalid shared library - Mailing list pgsql-hackers

On Wed, Feb 9, 2022 at 5:58 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> FYI, it has said "while..." and hasn't said "guc" since the 2nd revision of the
> patch.

The v3-0001 attached above had "while... for GUC..."--sorry I wasn't clear.

In v4, the message looks fine to me for shared_preload_libraries
(except there is a doubled "is"). However, I also get the message for
a simple SET with local_preload_libraries:

postgres=# set local_preload_libraries=xyz;
WARNING:  could not access file "xyz"
HINT:  The server will fail to start with the existing configuration.
If it is is shut down, it will be necessary to manually edit the
postgresql.auto.conf file to allow it to start.
SET

I'm not familiar with that setting (reading the docs, it's like a
non-superuser session_preload_libraries for compatible modules?), but
given nothing is being persisted here with ALTER SYSTEM, this seems
incorrect.

Changing session_preload_libraries emits a similar warning:

postgres=# set session_preload_libraries = foo;
WARNING:  could not access file "$libdir/plugins/foo"
HINT:  New sessions will fail with the existing configuration.
SET

This is also not persisted, so I think this is also incorrect, right?
(I'm not sure what setting session_preload_libraries without an ALTER
ROLE or ALTER DATABASE accomplishes, given a new session is required
for the change to take effect, but I thought I'd point this out.) I'm
guessing this may be due to trying to have the warning for ALTER ROLE?

postgres=# alter role bob set session_preload_libraries = foo;
WARNING:  could not access file "$libdir/plugins/foo"
HINT:  New sessions will fail with the existing configuration.
ALTER ROLE

This is great. Ideally, we'd qualify this with "New sessions for
user..." or "New sessions for database..." but given you get the
warning right after running the relevant command, maybe that's clear
enough.

> $ ./tmp_install/usr/local/pgsql/bin/postgres -D src/test/regress/tmp_check/data -clogging_collector=on
> 2022-02-09 19:53:48.034 CST postmaster[30979] FATAL:  could not access file "a": No such file or directory
> 2022-02-09 19:53:48.034 CST postmaster[30979] CONTEXT:  while loading shared libraries for setting
"shared_preload_libraries"
>         from /home/pryzbyj/src/postgres/src/test/regress/tmp_check/data/postgresql.auto.conf:3
> 2022-02-09 19:53:48.034 CST postmaster[30979] LOG:  database system is shut down
>
> Maybe it's enough to show the GucSource rather than file:line...

This is great. I think the file:line output is helpful here.

Thanks,
Maciek