On Tue, Dec 11, 2012 at 11:46 AM, Karl O. Pinc <kop@meme.com> wrote:
> Hi Josh,
>
> I've signed up to review this patch.
Thanks!
> I configured with assertions, built, and tested using
> the attached script. It seems to do what it's supposed
> to and the code looks ok to me.
>
> The docs build. The text is reasonable.
>
> I also diffed the output of the attached script with
> the output of an unpatched head and got what I expected.
Cool test script.
> Yes, the current pg_restore silently
> ignores multiple --table arguments, and seems to use the last
> one. You are introducing a backwards incompatible
> change here. I don't know what to do about it, other
> than perhaps to have the patch go into 10.0 (!?) and
> introduce a patch now that complains about multiple
> --table arguments. On the other hand, perhaps it's
> simply undocumented what pg_restore does when
> given repeated, conflicting, arguments and we're
> free to change this. Any thoughts?
Agreed with Robert that this change should be reasonable in a major
version (i.e. 9.3).
> On 12/10/2012 09:23:03 PM, Karl O. Pinc wrote:
>> On 10/30/2012 10:14:19 PM, Josh Kupershmidt wrote:
>>
>> > I went ahead and cooked up a patch to allow pg_restore, clusterdb,
>> > vacuumdb, and reindexdb to accept multiple --table switches. Hope I
>> > didn't overlook a similar tool, but these were the only other
>> > commands
>> > I found taking a --table argument.
>>
>> I believe you need ellipses behind --table in the syntax summaries
>> of the command reference docs.
Hrm, I was following pg_dump's lead here for the .sgml docs, and
didn't see anywhere that pg_dump makes the multiple --table syntax
explicit other than in the explanatory text underneath the option.
> I also note that the pg_dump --help output says "table(s)" so
> you probably want to have pg_restore say the same now that it
> takes multiple tables.
Good catch, will fix, and ditto reindexdb's --index help output. (It
is possible that the --help output for pg_dump was worded to say
"table(s)" because one can use a "pattern" --table specification with
pg_dump, though IMO it's helpful to mention "table(s)" in the --help
output for the rest of these programs as well, as a little reminder to
the user.)
Josh
