From: | "Karl O(dot) Pinc" <kop(at)meme(dot)com> |
---|---|
To: | Josh Kupershmidt <schmiddy(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Multiple --table options for other commands |
Date: | 2012-12-11 18:46:12 |
Message-ID: | 1355251572.14201.1@mofo |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Josh,
I've signed up to review this patch.
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.
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?
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.
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.
These two minor points seem to be all that needs fixing.
Regards,
Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Attachment | Content-Type | Size |
---|---|---|
tablearg.sh | application/x-shellscript | 4.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2012-12-11 19:11:58 | Re: [PATCH 02/14] Add support for a generic wal reading facility dubbed XLogReader |
Previous Message | Tom Lane | 2012-12-11 18:29:17 | Re: skipping context for RAISE statements - maybe obsolete? |