Re: [HACKERS] pg_restore -t should match views, matviews, and foreign tables
Craig Ringer cr...@2ndquadrant.com writes: I think you're right that sequences should be included by pg_restore since they are by pg_dump, though. So v3 patch attached. You forgot SEQUENCE SET :-(. I fixed that and adjusted the docs a bit more and committed. 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
Re: [HACKERS] pg_restore -t should match views, matviews, and foreign tables
Hi I am sending a review of this trivial patch. 1.This patch enables the possibility to restore only selected view, mat. view, foreign table or sequence. Currently the option -t works with tables only. All other relation like objects are quietly ignored. With this patch, the check on type is enhanced to allow other types described by pg_class system table. The implementation is trivial: +strcmp(te-desc, TABLE DATA) == 0 || +strcmp(te-desc, VIEW) == 0 || +strcmp(te-desc, FOREIGN TABLE) == 0 || +strcmp(te-desc, MATERIALIZED VIEW) == 0 || +strcmp(te-desc, MATERIALIZED VIEW DATA) == 0 || +strcmp(te-desc, SEQUENCE) == 0) 2. There was not any objections against this patch. 3. There was not any problem with patching and compilation. 4. This feature is good enough documented. There is opened question, if the related line should be changed? The current text is not 100% accurate, but it is short, and well readable and understandable. -S, --superuser=NAME superuser user name to use for disabling triggers -t, --table=NAME restore named table -T, --trigger=NAME restore named trigger 5. All tests passed 6. There are no tests. But pg_dump related sw has not any tests yet. I don't see any issues - this patch is really trivial without risks. It is working already on pg_dump side, so the fix on pg_restore side is natural. Regards Pavel 2015-04-01 5:01 GMT+02:00 Craig Ringer cr...@2ndquadrant.com: Following on from this -bugs post: http://www.postgresql.org/message-id/camsr+ygj50tvtvk4dbp66gajeoc0kap6kxfehaom+neqmhv...@mail.gmail.com this patch adds support for views, foreign tables, and materialised views to the pg_restore -t flag. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] pg_restore -t should match views, matviews, and foreign tables
On 8 April 2015 at 05:05, David G. Johnston david.g.johns...@gmail.com wrote: On Tue, Apr 7, 2015 at 1:33 PM, Tom Lane t...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@gmx.net writes: On 3/31/15 11:01 PM, Craig Ringer wrote: this patch adds support for views, foreign tables, and materialised views to the pg_restore -t flag. I think this is a good change. Any concerns? Are we happy with pg_dump/pg_restore not distinguishing these objects by type? It seems rather analogous to letting ALTER TABLE work on views etc. Personally I'm fine with this, but certainly some people have complained about that approach so far as ALTER is concerned. (But the implication would be that we'd need four distinct switches, which is not an outcome I favor.) The pg_dump documentation for the equivalent -t switch states: Dump only tables (or views or sequences or foreign tables) matching table Does pg_dump need to be updated to address materialized views here? The pg_dump code handles materialized views, the docs weren't updated. I added mention of them in the next rev of the patch to pg_restore. Does pg_restore need to be updated to address sequences here? I'd be against that if pg_dump didn't already behave the same way. Given that, yes, I think so. ISTM that the two should mirror each other. Ideally, yes, but the differences go much deeper than this. to get the equivalent of: pg_restore -n myschema -t sometable in pg_dump you need: pg_dump -t \myschema\.\sometable\ pg_dump -n myschema -t sometable is **not** equivalent. In fact, the -n is ignored, and -t will match using the search_path. so they're never really going to be the same, just similar enough to catch people out most of the time. I think you're right that sequences should be included by pg_restore since they are by pg_dump, though. So v3 patch attached. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services From dc8985d4aa5e995e5107fe9dcb65ec061dc378eb Mon Sep 17 00:00:00 2001 From: Craig Ringer cr...@2ndquadrant.com Date: Wed, 1 Apr 2015 10:46:29 +0800 Subject: [PATCH] pg_restore -t should select views, matviews, and foreign tables Currently pg_restore's '-t' option selects only tables, not other relations. It should be able to match anything that behaves like a relation in the relation namespace, anything that's interchangable with a table, including: * Normal relations * Views * Materialized views * Foreign tables * Sequences Sequences are matched because pg_dump -t matches them, even though their status as relations is really just an implementation detail. Indexes are not matched because pg_dump -t doesn't match them, and because they aren't really relations. TOAST tables aren't matched, they're implementation detail. See: http://www.postgresql.org/message-id/camsr+ygj50tvtvk4dbp66gajeoc0kap6kxfehaom+neqmhv...@mail.gmail.com --- doc/src/sgml/ref/pg_dump.sgml| 2 +- doc/src/sgml/ref/pg_restore.sgml | 25 ++--- src/bin/pg_dump/pg_backup_archiver.c | 7 ++- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index a6e7b08..7f7da9e 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -501,7 +501,7 @@ PostgreSQL documentation termoption--table=replaceable class=parametertable/replaceable/option/term listitem para -Dump only tables (or views or sequences or foreign tables) matching +Dump only tables (or views, sequences, foreign tables or materialized views) matching replaceable class=parametertable/replaceable. Multiple tables can be selected by writing multiple option-t/ switches. Also, the replaceable class=parametertable/replaceable parameter is diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml index 9f8dc00..9119e3e 100644 --- a/doc/src/sgml/ref/pg_restore.sgml +++ b/doc/src/sgml/ref/pg_restore.sgml @@ -405,9 +405,28 @@ termoption--table=replaceable class=parametertable/replaceable/option/term listitem para -Restore definition and/or data of named table only. Multiple tables -may be specified with multiple option-t/ switches. This can be -combined with the option-n/option option to specify a schema. +Restore definition and/or data of the named table (or other relation) +only. This flag matches views, materialized views and foreign tables as +well as ordinary tables. Multiple relations may be specified with +multiple option-t/ switches. This can be combined with the +option-n/option option to specify a schema. +note + para + When literal-t/literal is specified, + applicationpg_restore/application makes no attempt to restore any + other database objects that the
Re: [HACKERS] pg_restore -t should match views, matviews, and foreign tables
On 8 April 2015 at 04:33, Tom Lane t...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@gmx.net writes: On 3/31/15 11:01 PM, Craig Ringer wrote: this patch adds support for views, foreign tables, and materialised views to the pg_restore -t flag. I think this is a good change. Any concerns? Are we happy with pg_dump/pg_restore not distinguishing these objects by type? It seems rather analogous to letting ALTER TABLE work on views etc. Personally I'm fine with this, but certainly some people have complained about that approach so far as ALTER is concerned. (But the implication would be that we'd need four distinct switches, which is not an outcome I favor.) My reasoning was that these are all relations that, as far as SELECT et al are concerned, can be interchangeable. I guess this is more like the ALTER TABLE case though - if you pg_restore -t a view, you don't get the data from any table(s) it depends on. So substituting a table for a view won't be transparent to the user anyway. I mostly just don't see the point of requiring multiple flags for things that are all in the same namespace. It'll mean new flags each time we add some new object type, more logic in apps that invoke pg_restore, etc, and for what seems like no meaningful gain. We'll just land up with No table 'viewblah' matched, did you mean -V 'viewblah'? Also, I think you missed MATERIALIZED VIEW DATA. Thanks, amended. Also, shouldn't there be a documentation update? Yes. Again, amended. I've also added mention of materialized views to the pg_dump docs for --table, which omitted them. (It's rather unfortunate that pg_restore's -t is completely different to pg_dump's -t . Fixing that would involve implementing wildcard search support in pg_restore and would break backward compatibility, though). -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services From bccc5623f39a40a7ba3f63b3dcaf902259ad485c Mon Sep 17 00:00:00 2001 From: Craig Ringer cr...@2ndquadrant.com Date: Wed, 1 Apr 2015 10:46:29 +0800 Subject: [PATCH] pg_restore -t should select views, matviews, and foreign tables Currently pg_restore's '-t' option selects only tables, not other relations. It should be able to match anything that behaves like a relation in the relation namespace, anything that's interchangable with a table, including: * Normal relations * Views * Materialized views * Foreign tables Sequences are not matched. They're in the relation namespace, but only as an implementation detail. A separate option to selectively dump sequences should be added so that there's no BC break if they later become non-class objects. Indexes are also not matched; again, a different option should be added for them. TOAST tables aren't matched, they're implementation detail. See: http://www.postgresql.org/message-id/camsr+ygj50tvtvk4dbp66gajeoc0kap6kxfehaom+neqmhv...@mail.gmail.com --- doc/src/sgml/ref/pg_dump.sgml| 2 +- doc/src/sgml/ref/pg_restore.sgml | 25 ++--- src/bin/pg_dump/pg_backup_archiver.c | 6 +- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index a6e7b08..7f7da9e 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -501,7 +501,7 @@ PostgreSQL documentation termoption--table=replaceable class=parametertable/replaceable/option/term listitem para -Dump only tables (or views or sequences or foreign tables) matching +Dump only tables (or views, sequences, foreign tables or materialized views) matching replaceable class=parametertable/replaceable. Multiple tables can be selected by writing multiple option-t/ switches. Also, the replaceable class=parametertable/replaceable parameter is diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml index 9f8dc00..9119e3e 100644 --- a/doc/src/sgml/ref/pg_restore.sgml +++ b/doc/src/sgml/ref/pg_restore.sgml @@ -405,9 +405,28 @@ termoption--table=replaceable class=parametertable/replaceable/option/term listitem para -Restore definition and/or data of named table only. Multiple tables -may be specified with multiple option-t/ switches. This can be -combined with the option-n/option option to specify a schema. +Restore definition and/or data of the named table (or other relation) +only. This flag matches views, materialized views and foreign tables as +well as ordinary tables. Multiple relations may be specified with +multiple option-t/ switches. This can be combined with the +option-n/option option to specify a schema. +note + para + When literal-t/literal is specified, + applicationpg_restore/application makes no attempt to restore any + other database objects that the
Re: [HACKERS] pg_restore -t should match views, matviews, and foreign tables
Peter Eisentraut pete...@gmx.net writes: On 3/31/15 11:01 PM, Craig Ringer wrote: this patch adds support for views, foreign tables, and materialised views to the pg_restore -t flag. I think this is a good change. Any concerns? Are we happy with pg_dump/pg_restore not distinguishing these objects by type? It seems rather analogous to letting ALTER TABLE work on views etc. Personally I'm fine with this, but certainly some people have complained about that approach so far as ALTER is concerned. (But the implication would be that we'd need four distinct switches, which is not an outcome I favor.) Also, I think you missed MATERIALIZED VIEW DATA. Also, shouldn't there be a documentation update? 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
Re: [HACKERS] pg_restore -t should match views, matviews, and foreign tables
On 3/31/15 11:01 PM, Craig Ringer wrote: Following on from this -bugs post: http://www.postgresql.org/message-id/camsr+ygj50tvtvk4dbp66gajeoc0kap6kxfehaom+neqmhv...@mail.gmail.com this patch adds support for views, foreign tables, and materialised views to the pg_restore -t flag. I think this is a good change. Any concerns? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_restore -t should match views, matviews, and foreign tables
On Tue, Apr 7, 2015 at 1:33 PM, Tom Lane t...@sss.pgh.pa.us wrote: Peter Eisentraut pete...@gmx.net writes: On 3/31/15 11:01 PM, Craig Ringer wrote: this patch adds support for views, foreign tables, and materialised views to the pg_restore -t flag. I think this is a good change. Any concerns? Are we happy with pg_dump/pg_restore not distinguishing these objects by type? It seems rather analogous to letting ALTER TABLE work on views etc. Personally I'm fine with this, but certainly some people have complained about that approach so far as ALTER is concerned. (But the implication would be that we'd need four distinct switches, which is not an outcome I favor.) The pg_dump documentation for the equivalent -t switch states: Dump only tables (or views or sequences or foreign tables) matching table Does pg_dump need to be updated to address materialized views here? Does pg_restore need to be updated to address sequences here? ISTM that the two should mirror each other. David J.
[HACKERS] pg_restore -t should match views, matviews, and foreign tables
Following on from this -bugs post:
http://www.postgresql.org/message-id/camsr+ygj50tvtvk4dbp66gajeoc0kap6kxfehaom+neqmhv...@mail.gmail.com
this patch adds support for views, foreign tables, and materialised views
to the pg_restore -t flag.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 0319a7ecbab5c1e85e300d93f674087786be144a Mon Sep 17 00:00:00 2001
From: Craig Ringer cr...@2ndquadrant.com
Date: Wed, 1 Apr 2015 10:46:29 +0800
Subject: [PATCH] pg_restore -t should select views, matviews, and foreign
tables
Currently pg_restore's '-t' option selects only tables, not other
relations. It should be able to match anything that behaves like
a relation in the relation namespace, anything that's interchangable
with a table, including:
* Normal relations
* Views
* Materialized views
* Foreign tables
Sequences are not matched. They're in the relation namespace, but
only as an implementation detail. A separate option to selectively
dump sequences should be added so that there's no BC break if
they later become non-class objects.
Indexes are also not matched; again, a different option should be
added for them.
TOAST tables aren't matched, they're implementation detail.
See:
http://www.postgresql.org/message-id/camsr+ygj50tvtvk4dbp66gajeoc0kap6kxfehaom+neqmhv...@mail.gmail.com
---
src/bin/pg_dump/pg_backup_archiver.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index ca427de..75c8515 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -2663,7 +2663,10 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
if (ropt-selTypes)
{
if (strcmp(te-desc, TABLE) == 0 ||
- strcmp(te-desc, TABLE DATA) == 0)
+ strcmp(te-desc, TABLE DATA) == 0 ||
+ strcmp(te-desc, VIEW) == 0 ||
+ strcmp(te-desc, FOREIGN TABLE) == 0 ||
+ strcmp(te-desc, MATERIALIZED VIEW) == 0)
{
if (!ropt-selTable)
return 0;
--
2.1.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
