Re: should we document an example to set multiple libraries in shared_preload_libraries?

On Thu, Dec 9, 2021 at 1:02 PM Maciek Sakrejda <m(dot)sakrejda(at)gmail(dot)com> wrote:
>
> On Wed, Dec 1, 2021 at 5:15 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Justin Pryzby <pryzby(at)telsasoft(dot)com> writes:
> > > +1 to document it, but it seems like the worse problem is allowing the admin to
> > > write a configuration which causes the server to fail to start, without having
> > > issued a warning.
> >
> > > I think you could fix that with a GUC check hook to emit a warning.
> > > ...
> >
> > Considering the vanishingly small number of actual complaints we've
> > seen about this, that sounds ridiculously over-engineered.
> > A documentation example should be sufficient.
>
> I don't know if this will tip the scales, but I'd like to lodge a
> belated complaint. I've gotten myself in this server-fails-to-start
> situation several times (in development, for what it's worth). The
> syntax (as Bharath pointed out in the original message) is pretty
> picky, there are no guard rails, and if you got there through ALTER
> SYSTEM, you can't fix it with ALTER SYSTEM (because the server isn't
> up). If you go to fix it manually, you get a scary "Do not edit this
> file manually!" warning that you have to know to ignore in this case
> (that's if you find the file after you realize what the fairly generic
> "FATAL: ... No such file or directory" error in the log is telling
> you). Plus you have to get the (different!) quoting syntax right or
> cut your losses and delete the change.
>
> I'm over-dramatizing this a bit, but I do think there are a lot of
> opportunities to make mistakes here, and this behavior could be more
> user-friendly beyond just documentation changes. If a config change is
> bogus most likely due to a quoting mistake or a typo, a warning would
> be fantastic (i.e., the stat() check Justin suggested). Or maybe the
> FATAL log message on a failed startup could include the source of the
> problem?

How about ALTER SYSTEM SET shared_preload_libraries command itself
checking for availability of the specified libraries (after fetching
library names from the parsed string value) with stat() and then
report an error if any of the libraries doesn't exist? Currently, it
just accepts if the specified value passes the parsing.

Regards,
Bharath Rupireddy.