On 12/08/2011 09:18 PM, Joachim Wieland wrote:
> On Tue, Nov 15, 2011 at 6:14 PM, Andrew Dunstan<andrew@dunslane.net> wrote:
>> Updated version with pg_restore included is attached.
> The patch applies with some fuzz by now but compiles without errors or warnings.
>
> The feature just works, it is not adding a lot of new code, basically
> it parses the given options and then skips over steps depending on the
> selected section.
>
> I verified the equivalence of -a and -s to the respective sections in
> the different archive formats and no surprise here either, they were
> equivalent except for the header (which has a timestamp).
>
> If you ask pg_restore to restore a section out of an archive which
> doesn't have this section, there is no error and the command just
> succeeds. This is what I expected and I think it's the right thing to
> do but maybe others think that
> there should be a warning.
>
> In pg_restore, pre-data cannot be run in parallel, it would only run
> serially, data and post-data can run in parallel, though. This is also
> what I had expected but it might be worth to add a note about this to
> the documentation.
This is true now of parallel restore, and is by design (see debates from
the time.)
> What I didn't like about the implementation was the two set_section()
> functions, I'd prefer them to move to a file that is shared between
> pg_dump and pg_restore and become one function...
Done
>
> Minor issues:
>
> {"section", required_argument, NULL, 5} in pg_dump.c is not in the alphabetical
> order of the options.
>
> ./pg_restore --section=foobar
> pg_restore: unknown section name "foobar")
>
> Note the trailing ')', it's coming from a _(...) confusion
>
> Some of the lines in the patch have trailing spaces and in the
> documentation part tabs and spaces are mixed.
>
> int skip used as bool skip in dumpDumpableObject()
>
>
Should all be fixed. Revised patch attached.
cheers
andrew