On Wed, Dec 01, 2021 at 04:20:52PM +0530, Bharath Rupireddy wrote:
> It seems like users can try different ways to set multiple values for
> shared_preload_libraries GUC even after reading the documentation
> [1]), something like:
...
> ALTER SYSTEM SET shared_preload_libraries = "auth_delay,pg_stat_statements,sepgsql"; --> wrong
>
> The problem with the wrong parameter set command is that the ALTER
> SYSTEM SET will not fail, but the server will not come up in case it
> is restarted. In various locations in the documentation, we have shown
> how a single value can be set, something like:
> shared_preload_libraries = 'auth_delay'
> shared_preload_libraries = 'pg_stat_statements'
> shared_preload_libraries = 'sepgsql'
>
> Isn't it better we document (in [1]) an example to set multiple values
> to shared_preload_libraries?
+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.
I'm not sure what objections people might have to this. Maybe it's confusing
to execute preliminary verification of the library by calling stat() but not do
stronger verification for other reasons the library might fail to load. Like
it doesn't have the right magic number, or it's built for the wrong server
version. Should factor out the logic from internal_load_library and check
those too ?
--
Justin