Re: Conflicting option checking in pg_restore

On Sun, Oct 28, 2018 at 10:02:02PM +0100, Daniel Gustafsson wrote:
>> On 28 Oct 2018, at 19:42, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
>>>> Function RestoreArchive is called both from pg_dump & pg_restore, so now
>>>> the sanity check is not performed for the former (which does not have the
>>>> -1 option, though). Moreover, the function is noted "Public", which may
>>>> suggest that external tools could take advantage of it, and if so it
>>>> suggests that maybe it is not wise to remove the test. Any opinion around?

I would still need to see such a tool, and they would need most of the
stuff that pg_dump has already to get the object data.  As far as I know
it is marked as public because plain-text mode of pg_dump uses it.

>>> Wouldn't ropt->single_txn be undefined when called from pg_dump ?
>> 
>> Yes, probably.
> 
> pg_dump creates the RestoreOptions struct with NewRestoreOptions() which
> allocates it with pg_malloc0(), making single_txn false.

Which is why it would not be a problem for pg_dump.  The cross-option
check you are moving has been added as of 3a819b07, still you are right
that such checks should happen at the beginning so the suggestion of
this thread seems right to me.

Any objections from others?
--
Michael