On Thu, Oct 13, 2022 at 03:25:27PM +0530, Bharath Rupireddy wrote:
> The intent here looks reasonable to me. However, why should the user
> be able to set both archive_command and archive_library in the first
> place only to later fail in LoadArchiveLibrary() per the patch? IMO,
> the check_hook() is the right way to disallow any sorts of GUC
> misconfigurations, no?
There was some discussion upthread about using the GUC hooks to enforce
this [0]. In general, it doesn't seem to be a recommended practice. One
basic example of the problems with this approach is the following:
1. Set archive_command and leave archive_library unset and restart
the server.
2. Unset archive_command and set archive_library and call 'pg_ctl
reload'.
After these steps, you'll see the following log messages:
2022-10-13 10:58:42.112 PDT [1562524] LOG: received SIGHUP, reloading
configuration files
2022-10-13 10:58:42.114 PDT [1562524] LOG: cannot set
"archive_library" when "archive_command" is specified
2022-10-13 10:58:42.114 PDT [1562524] DETAIL: Only one of
"archive_library" or "archive_command" can be specified.
2022-10-13 10:58:42.114 PDT [1562524] LOG: parameter "archive_command"
changed to ""
2022-10-13 10:58:42.114 PDT [1562524] LOG: configuration file
"/home/nathan/pgdata/postgresql.conf" contains errors; unaffected changes were
applied
[0] https://postgr.es/m/20220914200305.GA2984249%40nathanxps13
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
