pg_restore multiple --function options

Started by Heikki Linnakangasover 12 years ago12 messageshackers
Jump to latest
#1Heikki Linnakangas
heikki.linnakangas@enterprisedb.com

Hi,

While looking at the pg_restore code, I noticed that while it supports
specifying multiple --table options to restore several tables, it does
not support multiple --function options. Or --index, --schema, or --trigger.

The support for multiple --table options was added in 9.3, in January.
See
/messages/by-id/CAK3UJRG+yV1mK5twLfKVMCwXH4f6PnJou6Rn=ecabyfQH1vVHg@mail.gmail.com.
Was there a particular reason for only doing it for --table, or was it
just an oversight or lack of interest? No doubt that --table is the most
interesting one, but IMHO the other options should behave the same, for
the sake of consistency.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2David Fetter
david@fetter.org
In reply to: Heikki Linnakangas (#1)
Re: pg_restore multiple --function options

On Mon, Aug 26, 2013 at 10:29:06PM +0300, Heikki Linnakangas wrote:

Hi,

While looking at the pg_restore code, I noticed that while it
supports specifying multiple --table options to restore several
tables, it does not support multiple --function options. Or --index,
--schema, or --trigger.

The support for multiple --table options was added in 9.3, in
January. See /messages/by-id/CAK3UJRG+yV1mK5twLfKVMCwXH4f6PnJou6Rn=ecabyfQH1vVHg@mail.gmail.com.
Was there a particular reason for only doing it for --table, or was
it just an oversight or lack of interest? No doubt that --table is
the most interesting one, but IMHO the other options should behave
the same, for the sake of consistency.

+1 for making them consistent. There will also be an improvement in
usability.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Michael Paquier
michael@paquier.xyz
In reply to: David Fetter (#2)
Re: pg_restore multiple --function options

On Tue, Aug 27, 2013 at 5:17 AM, David Fetter <david@fetter.org> wrote:

On Mon, Aug 26, 2013 at 10:29:06PM +0300, Heikki Linnakangas wrote:

Hi,

While looking at the pg_restore code, I noticed that while it
supports specifying multiple --table options to restore several
tables, it does not support multiple --function options. Or --index,
--schema, or --trigger.

The support for multiple --table options was added in 9.3, in
January. See /messages/by-id/CAK3UJRG+yV1mK5twLfKVMCwXH4f6PnJou6Rn=ecabyfQH1vVHg@mail.gmail.com.
Was there a particular reason for only doing it for --table, or was
it just an oversight or lack of interest? No doubt that --table is
the most interesting one, but IMHO the other options should behave
the same, for the sake of consistency.

+1 for making them consistent. There will also be an improvement in
usability.

+1. It would be good to have consistency there for all the objects.
-- 
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Michael Paquier (#3)
Re: pg_restore multiple --function options

On 27.08.2013 03:26, Michael Paquier wrote:

On Tue, Aug 27, 2013 at 5:17 AM, David Fetter<david@fetter.org> wrote:

On Mon, Aug 26, 2013 at 10:29:06PM +0300, Heikki Linnakangas wrote:

While looking at the pg_restore code, I noticed that while it
supports specifying multiple --table options to restore several
tables, it does not support multiple --function options. Or --index,
--schema, or --trigger.

The support for multiple --table options was added in 9.3, in
January. See /messages/by-id/CAK3UJRG+yV1mK5twLfKVMCwXH4f6PnJou6Rn=ecabyfQH1vVHg@mail.gmail.com.
Was there a particular reason for only doing it for --table, or was
it just an oversight or lack of interest? No doubt that --table is
the most interesting one, but IMHO the other options should behave
the same, for the sake of consistency.

+1 for making them consistent. There will also be an improvement in
usability.

+1. It would be good to have consistency there for all the objects.

Would anyone object to backpatching that change to 9.3 ? The risk seems
very small, and it would be good to do the other options in the same
release as --table. It was an oversight to only do it for --table in 9.3.

Assuming no objections, I'll apply the attached patch to 9.3 and master
later tonight.

- Heikki

Attachments:

multiple-pg_restore-options-1.patchtext/x-diff; name=multiple-pg_restore-options-1.patchDownload+21-17
#5David Fetter
david@fetter.org
In reply to: Heikki Linnakangas (#4)
Re: pg_restore multiple --function options

On Tue, Aug 27, 2013 at 08:14:32PM +0300, Heikki Linnakangas wrote:

On 27.08.2013 03:26, Michael Paquier wrote:

On Tue, Aug 27, 2013 at 5:17 AM, David Fetter<david@fetter.org> wrote:

On Mon, Aug 26, 2013 at 10:29:06PM +0300, Heikki Linnakangas wrote:

While looking at the pg_restore code, I noticed that while it
supports specifying multiple --table options to restore several
tables, it does not support multiple --function options. Or --index,
--schema, or --trigger.

The support for multiple --table options was added in 9.3, in
January. See /messages/by-id/CAK3UJRG+yV1mK5twLfKVMCwXH4f6PnJou6Rn=ecabyfQH1vVHg@mail.gmail.com.
Was there a particular reason for only doing it for --table, or was
it just an oversight or lack of interest? No doubt that --table is
the most interesting one, but IMHO the other options should behave
the same, for the sake of consistency.

+1 for making them consistent. There will also be an improvement in
usability.

+1. It would be good to have consistency there for all the objects.

Would anyone object to backpatching that change to 9.3 ?

I'm sure someone will, but I'm not that someone. It's not as though
the added syntax could have produced anything but a runtime error if
someone had it deployed, and the improvements both in usefulness and
consistency are compelling.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Heikki Linnakangas (#4)
Re: pg_restore multiple --function options

Heikki Linnakangas <hlinnakangas@vmware.com> writes:

On 27.08.2013 03:26, Michael Paquier wrote:

On Tue, Aug 27, 2013 at 5:17 AM, David Fetter<david@fetter.org> wrote:

On Mon, Aug 26, 2013 at 10:29:06PM +0300, Heikki Linnakangas wrote:

While looking at the pg_restore code, I noticed that while it
supports specifying multiple --table options to restore several
tables, it does not support multiple --function options. Or --index,
--schema, or --trigger.

The support for multiple --table options was added in 9.3, in
January. See /messages/by-id/CAK3UJRG+yV1mK5twLfKVMCwXH4f6PnJou6Rn=ecabyfQH1vVHg@mail.gmail.com.
Was there a particular reason for only doing it for --table, or was
it just an oversight or lack of interest? No doubt that --table is
the most interesting one, but IMHO the other options should behave
the same, for the sake of consistency.

+1 for making them consistent. There will also be an improvement in
usability.

+1. It would be good to have consistency there for all the objects.

Would anyone object to backpatching that change to 9.3 ? The risk seems
very small, and it would be good to do the other options in the same
release as --table. It was an oversight to only do it for --table in 9.3.

Assuming no objections, I'll apply the attached patch to 9.3 and master
later tonight.

I object, strongly. This is a feature addition, and has no business going
in post-rc1, especially with no time for review.

As far as the function case goes, I'm not really thrilled about layering
more functionality on that until we've come to some understanding about
how to select from a group of overloaded functions. It may be that this
is orthogonal to that issue ... or maybe not. I don't have any objection
to fixing the non-function cases, as long as it's only in HEAD.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Tom Lane (#6)
Re: pg_restore multiple --function options

On 27.08.2013 21:56, Tom Lane wrote:

Heikki Linnakangas<hlinnakangas@vmware.com> writes:

Would anyone object to backpatching that change to 9.3 ? The risk seems
very small, and it would be good to do the other options in the same
release as --table. It was an oversight to only do it for --table in 9.3.

Assuming no objections, I'll apply the attached patch to 9.3 and master
later tonight.

I object, strongly. This is a feature addition, and has no business going
in post-rc1, especially with no time for review.

Ok.

As far as the function case goes, I'm not really thrilled about layering
more functionality on that until we've come to some understanding about
how to select from a group of overloaded functions. It may be that this
is orthogonal to that issue ... or maybe not. I don't have any objection
to fixing the non-function cases, as long as it's only in HEAD.

Huh, what's that issue?

As the code stands, you have to pass the argument types to the
--function flag, ie. --function="myfunc(integer)". It's annoyingly picky
about the spelling, as the it has to match exactly what pg_dump prints,
but it does handle selecting one function from a group of overloaded
ones. And that really is orthogonal to whether or not you can give
multiple --function arguments.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Josh Berkus
josh@agliodbs.com
In reply to: Heikki Linnakangas (#1)
Re: pg_restore multiple --function options

On 08/27/2013 10:14 AM, Heikki Linnakangas wrote:

Would anyone object to backpatching that change to 9.3 ? The risk seems
very small, and it would be good to do the other options in the same
release as --table. It was an oversight to only do it for --table in 9.3.

Assuming no objections, I'll apply the attached patch to 9.3 and master
later tonight.

That will pretty much force us to do an RC2, and therefore push back
final release date.

Just for consideration.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Heikki Linnakangas (#7)
Re: pg_restore multiple --function options

Heikki Linnakangas <hlinnakangas@vmware.com> writes:

As the code stands, you have to pass the argument types to the
--function flag, ie. --function="myfunc(integer)". It's annoyingly picky
about the spelling, as the it has to match exactly what pg_dump prints,
but it does handle selecting one function from a group of overloaded
ones.

Oh --- OK, I was misremembering. I recalled that people weren't happy
with the handling of --function, but had the details wrong.

[pokes around]

I think really the issues are (1) it only works in pg_restore, not
pg_dump, and (2) there's no wildcard matching (the pickiness about
argument type name spelling being perhaps a subset of that).

It's probably true that accepting multiple patterns doesn't preclude
solving either of those, and indeed might help users work around (2).
So nevermind that objection. But I still say this is all too late
for 9.3.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10David Fetter
david@fetter.org
In reply to: Heikki Linnakangas (#7)
Re: pg_restore multiple --function options

On Tue, Aug 27, 2013 at 10:35:40PM +0300, Heikki Linnakangas wrote:

On 27.08.2013 21:56, Tom Lane wrote:

Heikki Linnakangas<hlinnakangas@vmware.com> writes:

Would anyone object to backpatching that change to 9.3 ? The risk seems
very small, and it would be good to do the other options in the same
release as --table. It was an oversight to only do it for --table in 9.3.

Assuming no objections, I'll apply the attached patch to 9.3 and master
later tonight.

I object, strongly. This is a feature addition, and has no business going
in post-rc1, especially with no time for review.

Ok.

As far as the function case goes, I'm not really thrilled about layering
more functionality on that until we've come to some understanding about
how to select from a group of overloaded functions. It may be that this
is orthogonal to that issue ... or maybe not. I don't have any objection
to fixing the non-function cases, as long as it's only in HEAD.

Huh, what's that issue?

As the code stands, you have to pass the argument types to the
--function flag, ie. --function="myfunc(integer)". It's annoyingly
picky about the spelling, as the it has to match exactly what
pg_dump prints, but it does handle selecting one function from a
group of overloaded ones. And that really is orthogonal to whether
or not you can give multiple --function arguments.

Come to think of it, some kind of recognition that functions can come
in several flavors would be awesome, e.g.

--function=myfunc\*

which would capture all variants of myfunc.

Let the bikeshedding begin!

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Josh Kupershmidt
schmiddy@gmail.com
In reply to: Heikki Linnakangas (#4)
Re: pg_restore multiple --function options

On Tue, Aug 27, 2013 at 1:14 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:

Assuming no objections, I'll apply the attached patch to 9.3 and master
later tonight.

Just a little stylistic nitpick: could we pluralize the --help outputs
for the modified options so that they make clear that multiple
specifications are supported. E.g. "restore named index(es)" instead
of just "restore named index". The last round of such changes tried to
make such --help tweaks, it would be nice to keep 'em consistent.

Josh

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Bruce Momjian
bruce@momjian.us
In reply to: Josh Kupershmidt (#11)
Re: pg_restore multiple --function options

On Thu, Sep 5, 2013 at 09:15:09PM -0400, Josh Kupershmidt wrote:

On Tue, Aug 27, 2013 at 1:14 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:

Assuming no objections, I'll apply the attached patch to 9.3 and master
later tonight.

Just a little stylistic nitpick: could we pluralize the --help outputs
for the modified options so that they make clear that multiple
specifications are supported. E.g. "restore named index(es)" instead
of just "restore named index". The last round of such changes tried to
make such --help tweaks, it would be nice to keep 'em consistent.

Agreed. Attached patch applied.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Attachments:

restore.difftext/x-diff; charset=us-asciiDownload+6-6