Re: Conflicting option checking in pg_restore

2018-10-29 Thread Michael Paquier
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

Re: Conflicting option checking in pg_restore

2018-10-29 Thread Fabien COELHO
Hallå Daniel, an assertion? v2 applies, compiles, both local & global make check are ok. Its CF category could have been "bug fix" or "restructuring". [...] Perhaps, we don’t really test for all other potential misconfigurations of the options so I can go either way. It’s also a cheap

Re: Conflicting option checking in pg_restore

2018-10-28 Thread Michael Paquier
On Sun, Oct 28, 2018 at 10:02:02PM +0100, Daniel Gustafsson wrote: >> On 28 Oct 2018, at 19:42, Fabien COELHO 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,

Re: Conflicting option checking in pg_restore

2018-10-28 Thread Daniel Gustafsson
> On 28 Oct 2018, at 19:42, Fabien COELHO 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

Re: Conflicting option checking in pg_restore

2018-10-28 Thread Daniel Gustafsson
> Could you add the patch to the CF app? > > https://commitfest.postgresql.org/20/ Done. >> Checking for conflicting options in pg_restore was mostly done in main() with >> one check deferred until RestoreArchive(). Reading the git history makes it >> seem like it simply happened, without the

Re: Conflicting option checking in pg_restore

2018-10-28 Thread Fabien COELHO
Hello Narayanan, There is a possible catch: 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

Re: Conflicting option checking in pg_restore

2018-10-28 Thread Narayanan V
Hi On Sun, Oct 28, 2018 at 12:25 PM Fabien COELHO wrote: > > Hallå Daniel, > > > Checking for conflicting options in pg_restore was mostly done in main() > with > > one check deferred until RestoreArchive(). Reading the git history > makes it > > seem like it simply happened, without the

Re: Conflicting option checking in pg_restore

2018-10-28 Thread Fabien COELHO
Hallå Daniel, Checking for conflicting options in pg_restore was mostly done in main() with one check deferred until RestoreArchive(). Reading the git history makes it seem like it simply happened, without the disjoint checking being intentional. Am I reading it right that we can consolidate

Conflicting option checking in pg_restore

2018-10-27 Thread Daniel Gustafsson
Checking for conflicting options in pg_restore was mostly done in main() with one check deferred until RestoreArchive(). Reading the git history makes it seem like it simply happened, without the disjoint checking being intentional. Am I reading it right that we can consolidate all the option