Re: Conflicting option checking in pg_restore

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Conflicting option checking in pg_restore
Date: 2018-10-30 02:39:31
Message-ID: 20181030023931.GC1644@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 29, 2018 at 07:11:35AM +0100, Fabien COELHO wrote:
> Michaƫl suggests that there is no issue of external tool using the internal
> function, so I'm fine with this version.
>
> I have switched the patch to ready for committer.

One catch with this refactoring is that for example this combination
does not result in an error on HEAD, but it does with the patch:
pg_restore -l -C -1

Anyway, the fact that we save the caller from one exit_horribly()
knowing that opening the archive is completely useless makes the move
worth it in my opinion. RestoreArchive should complain about things
which depend on the opened archive, which is the only thing it does
now. I would not risk back-patching it though.

At the same time, I have checked the set of TAP tests for pg_restore and
the cross-option checks are all covered, so no need to go crazy on this
side.

For the archive's sake, the original commit 3a819b07 which did the
option check in RestoreArchive comes from here:
https://www.postgresql.org/message-id/496B6B40.1010909@hagander.net

And committed, thanks Daniel and Fabien!
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-10-30 02:51:09 Re: replication_slots usability issue
Previous Message Amit Langote 2018-10-30 01:53:43 Re: partition tree inspection functions