From: Tom Lane <tgl@sss.pgh.pa.us>
To: Nathan Bossart <nathandbossart@gmail.com>
cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
        Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
        Benoit =?iso-8859-1?Q?Lobr=E9au?= <benoit.lobreau@gmail.com>,
        Alvaro Herrera <alvherre@alvh.no-ip.org>,
        Robert Haas <robertmhaas@gmail.com>,
        "Bossart,
    Nathan" <bossartn@amazon.com>,
        David Steele <david@pgmasters.net>,
        Fujii Masao <masao.fujii@oss.nttdata.com>,
        "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
        Michael Paquier <michael@paquier.xyz>,
        Magnus Hagander <magnus@hagander.net>,
        Stephen Frost <sfrost@snowman.net>
Subject: Re: archive modules
In-reply-to: <20221013182408.GA1551106@nathanxps13>
References: <2884265.1663188443@sss.pgh.pa.us>
 <20220914211746.GA2986134@nathanxps13> <2910705.1663193529@sss.pgh.pa.us>
 <20220914222736.GA3042279@nathanxps13>
 <d9d1f7fd-dd99-98f9-8aed-0e976921573b@enterprisedb.com>
 <20220923161402.GA736566@nathanxps13>
 <4f66de67-35a0-3fe7-fa61-d618e1bd2c2c@enterprisedb.com>
 <20221005185716.GB201192@nathanxps13>
 <1d9cb989-91b0-919e-f42b-4788f5fc7911@enterprisedb.com>
 <CALj2ACWAq_gTQ-FhjFvGjhOd5+7qWdepVmYcqsZ_whENshg1zw@mail.gmail.com>
 <20221013182408.GA1551106@nathanxps13>
Comments: In-reply-to Nathan Bossart <nathandbossart@gmail.com>
	message dated "Thu, 13 Oct 2022 11:24:08 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1061702.1665687218.1@sss.pgh.pa.us>
Date: Thu, 13 Oct 2022 14:53:38 -0400
Message-ID: <1061703.1665687218@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1061703.1665687218%40sss.pgh.pa.us>
Precedence: bulk

Nathan Bossart <nathandbossart@gmail.com> writes:
> 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.

Yeah, it really does not work to use GUC hooks to enforce multi-variable
constraints.  We've learned that the hard way (more than once, if memory
serves).

			regards, tom lane