Re: Option to dump foreign data in pg_dump
Hi everyone, I am just responding on the latest mail on this thread. But the question is about functionality. The proposal is to add a single flag --include-foreign-data which controls whether or not data is dumped for all the foreign tables in a database. That may not serve the purpose. A foreign table may point to a view, materialized view or inheritance tree, and so on. A database can have foreign tables pointing to all of those kinds. Restoring data to a view won't be possible and restoring it into an inheritance tree would insert it into the parent only and not the children. Further, a user may not want the data to be dumped for all the foreign tables since their usages are different esp. considering restore. I think a better option is to extract data in a foreign table using --table if that's the only usage. Otherwise, we need a foreign table level flag indicating whether pg_dump should dump the data for that foreign table or not. The option enables the user to dump data of tables backed by a specific foreign_server. It is up to the user to guarantee that the foreign server is also writable, that is the reason to make the option opt-in. The option can be combined with --table to dump specific tables if needed. If the user has different foreign servers in the database has to make the conscious decision of dumping each one of them. Without this option the user is totally unable to do it. > On 2020-01-21 10:36, Luis Carril wrote: >>> Yes we can support --include-foreign-data without parallel option and >>> later add support for parallel option as a different patch. >> >> Hi, >> >> I've attached a new version of the patch in which an error is >> emitted if the parallel backup is used with the --include-foreign-data >> option. > > This seems like an overreaction. The whole point of > lockTableForWorker() is to avoid deadlocks, but foreign tables don't > have locks, so it's not a problem. I think you can just skip foreign > tables in lockTableForWorker() using the same logic that getTables() uses. > > I think parallel data dump would be an especially interesting option > when using foreign tables, so it's worth figuring this out. What do you think of Peter's comment? I took a look at it, we could skip foreign tables by checking the catalog in lockTableForWorker but this would imply an extra query per call to the function (as in getTables), which would be irrelevant for most of the cases. Or we could pass in the TocEntry that it is a foreign table (although that seems highly specific). Also, would it not be possible to offer support of LOCK TABLE on foreign tables? At this point I would like to leave the patch as is, and discuss further improvement in a future patch. Luis M. From: Ashutosh Bapat Sent: Wednesday, March 4, 2020 5:39 PM To: David Steele Cc: Luis Carril ; vignesh C ; Peter Eisentraut ; Alvaro Herrera ; Daniel Gustafsson ; Laurenz Albe ; PostgreSQL Hackers Subject: Re: Option to dump foreign data in pg_dump I am just responding on the latest mail on this thread. But the question is about functionality. The proposal is to add a single flag --include-foreign-data which controls whether or not data is dumped for all the foreign tables in a database. That may not serve the purpose. A foreign table may point to a view, materialized view or inheritance tree, and so on. A database can have foreign tables pointing to all of those kinds. Restoring data to a view won't be possible and restoring it into an inheritance tree would insert it into the parent only and not the children. Further, a user may not want the data to be dumped for all the foreign tables since their usages are different esp. considering restore. I think a better option is to extract data in a foreign table using --table if that's the only usage. Otherwise, we need a foreign table level flag indicating whether pg_dump should dump the data for that foreign table or not. On Wed, Mar 4, 2020 at 12:41 AM David Steele mailto:da...@pgmasters.net>> wrote: Hi Luis, On 1/29/20 11:05 AM, Peter Eisentraut wrote: > On 2020-01-21 10:36, Luis Carril wrote: >>> Yes we can support --include-foreign-data without parallel option and >>> later add support for parallel option as a different patch. >> >> Hi, >> >> I've attached a new version of the patch in which an error is >> emitted if the parallel backup is used with the --include-foreign-data >> option. > > This seems like an overreaction. The whole point of > lockTableForWorker() is to avoid deadlocks, but foreign tables don't > have locks, so it's not a problem. I think you can just skip foreign > tables in lockTableForWorker() using the same logic that getTables() uses. > > I think parallel data dump would be an especially inte
Re: Option to dump foreign data in pg_dump
Thanks for working on the comments. I noticed one behavior is different when --table option is specified. When --table is specified the following are not getting dumped: CREATE SERVER foreign_server I felt the above also should be included as part of the dump when include-foreign-data option is specified. Yes, it also happens on master. A dump of a foreign table using --table, which only dumps the table definition, does not include the extension nor the server. I guess that the idea behind --table is that the table prerequisites should already exist on the database. A similar behavior can be reproduced for a non foreign table. If a table is created in a specific schema, dumping only the table with --table does not dump the schema definition. So I think we do not need to dump the server with the table. Cheers Luis M Carril
Re: Option to dump foreign data in pg_dump
Yes we can support --include-foreign-data without parallel option and
later add support for parallel option as a different patch.
Hi,
I've attached a new version of the patch in which an error is emitted if
the parallel backup is used with the --include-foreign-data option.
Cheers
Luis M. Carril
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 4bcd4bdaef..319851b936 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -767,6 +767,34 @@ PostgreSQL documentation
+
+ --include-foreign-data=foreignserver
+
+
+Dump the data for any foreign table with a foreign server
+matching foreignserver
+pattern. Multiple foreign servers can be selected by writing multiple --include-foreign-data.
+Also, the foreignserver parameter is
+interpreted as a pattern according to the same rules used by
+psql's \d commands (see ),
+so multiple foreign servers can also be selected by writing wildcard characters
+in the pattern. When using wildcards, be careful to quote the pattern
+if needed to prevent the shell from expanding the wildcards; see
+.
+The only exception is that an empty pattern is disallowed.
+
+
+
+
+ When --include-foreign-data is specified, pg_dump
+ does not check if the foreign table is also writeable. Therefore, there is no guarantee that
+ the results of a foreign table dump can be successfully restored by themselves into a clean database.
+
+
+
+
+
--inserts
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 8c0cedcd98..d255162c7a 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -141,6 +141,7 @@ typedef struct _dumpOptions
bool aclsSkip;
const char *lockWaitTimeout;
int dump_inserts; /* 0 = COPY, otherwise rows per INSERT */
+ bool include_foreign_data;
/* flags for various command-line long options */
int disable_dollar_quoting;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 799b6988b7..13156357dd 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -120,6 +120,8 @@ static SimpleStringList table_exclude_patterns = {NULL, NULL};
static SimpleOidList table_exclude_oids = {NULL, NULL};
static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
+static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
+static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
char g_opaque_type[10]; /* name for the opaque type */
@@ -156,6 +158,9 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_foreign_server_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
@@ -388,6 +393,7 @@ main(int argc, char **argv)
{"no-sync", no_argument, NULL, 7},
{"on-conflict-do-nothing", no_argument, _nothing, 1},
{"rows-per-insert", required_argument, NULL, 10},
+ {"include-foreign-data", required_argument, NULL, 11},
{NULL, 0, NULL, 0}
};
@@ -604,6 +610,13 @@ main(int argc, char **argv)
dopt.dump_inserts = (int) rowsPerInsert;
break;
+ case 11:/* include foreign data */
+if (strcmp(optarg, "") == 0)
+ fatal("empty string is not a valid pattern in --include-foreign-data");
+simple_string_list_append(_servers_include_patterns, optarg);
+dopt.include_foreign_data = true;
+break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -645,6 +658,12 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (dopt.schemaOnly && foreign_servers_include_patterns.head != NULL)
+ fatal("options -s/--schema-only and --include-foreign-data cannot be used together");
+
+ if (numWorkers > 1 && foreign_servers_include_patterns.head != NULL)
+ fatal("option --include-foreign-data is not supported with parallel backup");
+
if (dopt.dataOnly && dopt.outputClean)
{
pg_log_error("options -c/--clean and -a/--data-only cannot be used together");
@@ -812,6 +831,9 @@ main(int argc, char **argv)
_exclude_oids,
false);
+ expand_foreign_server_name_patterns(fout, _servers_include_patterns,
+ _servers_include_oids);
+
/* non-matching exclusion patterns aren't an error */
/*
@@ -1035,6 +1057,9 @@ help(const char *progname)
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
"
Re: Option to dump foreign data in pg_dump
On Tue, Jan 14, 2020 at 5:22 PM Luis Carril mailto:luis.car...@swarm64.com>> wrote: Can you have a look at dump with parallel option. Parallel option will take a lock on table while invoking lockTableForWorker. May be this is not required for foreign tables. Thoughts? I tried with -j and found no issue. I guess that the foreign table needs locking anyway to prevent anyone to modify it while is being dumped. I'm able to get the problem with the following steps: Bring up a postgres setup with servers running in 5432 & 5433 port. Execute the following commands in Server1 configured on 5432 port: * CREATE EXTENSION postgres_fdw; * CREATE SERVER foreign_server FOREIGN DATA WRAPPER postgres_fdw OPTIONS (host '127.0.0.1', port '5433', dbname 'postgres'); * create user user1 password '123'; * alter user user1 with superuser; * CREATE USER MAPPING FOR user1 SERVER foreign_server OPTIONS (user 'user1', password '123'); Execute the following commands in Server2 configured on 5433 port: * create user user1 password '123'; * alter user user1 with superuser; Execute the following commands in Server2 configured on 5433 port as user1 user: * create schema test; * create table test.test1(id int); * insert into test.test1 values(10); Execute the following commands in Server1 configured on 5432 port as user1 user: * CREATE FOREIGN TABLE foreign_table1 (id integer NOT NULL) SERVER foreign_server OPTIONS (schema_name 'test', table_name 'test1'); Without parallel option, the operation is successful: * ./pg_dump -d postgres -f dumpdir -U user1 -F d --include-foreign-data foreign_server With parallel option it fails: * ./pg_dump -d postgres -f dumpdir1 -U user1 -F d -j 5 --include-foreign-data foreign_server pg_dump: error: could not obtain lock on relation "public.foreign_table1" This usually means that someone requested an ACCESS EXCLUSIVE lock on the table after the pg_dump parent process had gotten the initial ACCESS SHARE lock on the table. pg_dump: error: a worker process died unexpectedly There may be simpler steps than this to reproduce the issue, i have not try to optimize it. Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com Hi Vignesh, yes you are right I could reproduce it also with 'file_fdw'. The issue is that LOCK is not supported on foreign tables, so I guess that the safest solution is to make the --include-foreign-data incompatible with --jobs, because skipping the locking for foreign tables maybe can lead to a deadlock anyway. Suggestions? Cheers Luis M Carril From: vignesh C Sent: Thursday, January 16, 2020 10:01 AM To: Luis Carril Cc: Alvaro Herrera ; Daniel Gustafsson ; Laurenz Albe ; PostgreSQL Hackers Subject: Re: Option to dump foreign data in pg_dump On Tue, Jan 14, 2020 at 5:22 PM Luis Carril mailto:luis.car...@swarm64.com>> wrote: Can you have a look at dump with parallel option. Parallel option will take a lock on table while invoking lockTableForWorker. May be this is not required for foreign tables. Thoughts? I tried with -j and found no issue. I guess that the foreign table needs locking anyway to prevent anyone to modify it while is being dumped. I'm able to get the problem with the following steps: Bring up a postgres setup with servers running in 5432 & 5433 port. Execute the following commands in Server1 configured on 5432 port: * CREATE EXTENSION postgres_fdw; * CREATE SERVER foreign_server FOREIGN DATA WRAPPER postgres_fdw OPTIONS (host '127.0.0.1', port '5433', dbname 'postgres'); * create user user1 password '123'; * alter user user1 with superuser; * CREATE USER MAPPING FOR user1 SERVER foreign_server OPTIONS (user 'user1', password '123'); Execute the following commands in Server2 configured on 5433 port: * create user user1 password '123'; * alter user user1 with superuser; Execute the following commands in Server2 configured on 5433 port as user1 user: * create schema test; * create table test.test1(id int); * insert into test.test1 values(10); Execute the following commands in Server1 configured on 5432 port as user1 user: * CREATE FOREIGN TABLE foreign_table1 (id integer NOT NULL) SERVER foreign_server OPTIONS (schema_name 'test', table_name 'test1'); Without parallel option, the operation is successful: * ./pg_dump -d postgres -f dumpdir -U user1 -F d --include-foreign-data foreign_server With parallel option it fails: * ./pg_dump -d postgres -f dumpdir1 -U user1 -F d -j 5 --include-foreign-data foreign_server pg_dump: error: could not obtain lock on relation "public.foreign_table1" This usually means that someone requested an ACCESS EXCLUSIVE lock on the table after the pg_dump parent process had gotten the initial ACCESS SHARE lock on the table. pg_dump: error: a worker process died unexpectedly There may
Re: Option to dump foreign data in pg_dump
Can you have a look at dump with parallel option. Parallel option will take a lock on table while invoking lockTableForWorker. May be this is not required for foreign tables. Thoughts? I tried with -j and found no issue. I guess that the foreign table needs locking anyway to prevent anyone to modify it while is being dumped. Cheers, Luis M Carril From: vignesh C Sent: Tuesday, January 14, 2020 1:48 AM To: Luis Carril Cc: Alvaro Herrera ; Daniel Gustafsson ; Laurenz Albe ; PostgreSQL Hackers Subject: Re: Option to dump foreign data in pg_dump On Fri, Nov 29, 2019 at 2:10 PM Luis Carril wrote: > > Luis, > > It seems you've got enough support for this concept, so let's move > forward with this patch. There are some comments from Tom about the > patch; would you like to send an updated version perhaps? > > Thanks > > Hi, > > I've attached a new version (v6) removing the superfluous JOIN that Tom > identified, and not collecting the oids (avoiding the query) if the option is > not used at all. > > About the testing issues that Tom mentioned: > I do not see how can we have a pure SQL dummy FDW that tests the > functionality. Because the only way to identify if the data of a foreign > table for the chosen server is dumped is if the COPY statement appears in the > output, but if the C callbacks of the FDW are not implemented, then the > SELECT that dumps the data to generate the COPY cannot be executed. > Also, to test that the include option chooses only the data of the specified > foreign servers we would need some negative testing, i.e. that the COPY > statement for the non-desired table does not appear. But I do not find these > kind of tests in the test suite, even for other selective options like > --table or --exclude-schema. > Can you have a look at dump with parallel option. Parallel option will take a lock on table while invoking lockTableForWorker. May be this is not required for foreign tables. Thoughts? Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com
Re: Option to dump foreign data in pg_dump
Luis,
It seems you've got enough support for this concept, so let's move
forward with this patch. There are some comments from Tom about the
patch; would you like to send an updated version perhaps?
Thanks
Hi,
I've attached a new version (v6) removing the superfluous JOIN that Tom
identified, and not collecting the oids (avoiding the query) if the option is
not used at all.
About the testing issues that Tom mentioned:
I do not see how can we have a pure SQL dummy FDW that tests the functionality.
Because the only way to identify if the data of a foreign table for the chosen
server is dumped is if the COPY statement appears in the output, but if the C
callbacks of the FDW are not implemented, then the SELECT that dumps the data
to generate the COPY cannot be executed.
Also, to test that the include option chooses only the data of the specified
foreign servers we would need some negative testing, i.e. that the COPY
statement for the non-desired table does not appear. But I do not find these
kind of tests in the test suite, even for other selective options like --table
or --exclude-schema.
Cheers
Luis M Carril
From: Alvaro Herrera
Sent: Thursday, November 28, 2019 3:31 PM
To: Luis Carril
Cc: Daniel Gustafsson ; Laurenz Albe
; vignesh C ; PostgreSQL Hackers
Subject: Re: Option to dump foreign data in pg_dump
On 2019-Nov-12, Luis Carril wrote:
> The nitpicks have been addressed. However, it seems that the new file
> containing the test FDW seems missing from the new version of the patch. Did
> you forget to git add the file?
>
> Yes, I forgot, thanks for noticing. New patch attached again.
Luis,
It seems you've got enough support for this concept, so let's move
forward with this patch. There are some comments from Tom about the
patch; would you like to send an updated version perhaps?
Thanks
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 4bcd4bdaef..319851b936 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -767,6 +767,34 @@ PostgreSQL documentation
+
+ --include-foreign-data=foreignserver
+
+
+Dump the data for any foreign table with a foreign server
+matching foreignserver
+pattern. Multiple foreign servers can be selected by writing multiple --include-foreign-data.
+Also, the foreignserver parameter is
+interpreted as a pattern according to the same rules used by
+psql's \d commands (see ),
+so multiple foreign servers can also be selected by writing wildcard characters
+in the pattern. When using wildcards, be careful to quote the pattern
+if needed to prevent the shell from expanding the wildcards; see
+.
+The only exception is that an empty pattern is disallowed.
+
+
+
+
+ When --include-foreign-data is specified, pg_dump
+ does not check if the foreign table is also writeable. Therefore, there is no guarantee that
+ the results of a foreign table dump can be successfully restored by themselves into a clean database.
+
+
+
+
+
--inserts
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 8c0cedcd98..d255162c7a 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -141,6 +141,7 @@ typedef struct _dumpOptions
bool aclsSkip;
const char *lockWaitTimeout;
int dump_inserts; /* 0 = COPY, otherwise rows per INSERT */
+ bool include_foreign_data;
/* flags for various command-line long options */
int disable_dollar_quoting;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bf69adc2f4..c0258c5c99 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -120,6 +120,8 @@ static SimpleStringList table_exclude_patterns = {NULL, NULL};
static SimpleOidList table_exclude_oids = {NULL, NULL};
static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
+static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
+static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
char g_opaque_type[10]; /* name for the opaque type */
@@ -156,6 +158,9 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_foreign_server_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
@@ -388,6 +393,7 @@ main(int argc, char **argv)
{"no-syn
Re: Option to dump foreign data in pg_dump
The nitpicks have been addressed. However, it seems that the new file
containing the test FDW seems missing from the new version of the patch. Did
you forget to git add the file?
Yes, I forgot, thanks for noticing. New patch attached again.
Cheers
Luis M Carril
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 4bcd4bdaef..319851b936 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -767,6 +767,34 @@ PostgreSQL documentation
+
+ --include-foreign-data=foreignserver
+
+
+Dump the data for any foreign table with a foreign server
+matching foreignserver
+pattern. Multiple foreign servers can be selected by writing multiple --include-foreign-data.
+Also, the foreignserver parameter is
+interpreted as a pattern according to the same rules used by
+psql's \d commands (see ),
+so multiple foreign servers can also be selected by writing wildcard characters
+in the pattern. When using wildcards, be careful to quote the pattern
+if needed to prevent the shell from expanding the wildcards; see
+.
+The only exception is that an empty pattern is disallowed.
+
+
+
+
+ When --include-foreign-data is specified, pg_dump
+ does not check if the foreign table is also writeable. Therefore, there is no guarantee that
+ the results of a foreign table dump can be successfully restored by themselves into a clean database.
+
+
+
+
+
--inserts
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bf69adc2f4..31465dec79 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -120,6 +120,8 @@ static SimpleStringList table_exclude_patterns = {NULL, NULL};
static SimpleOidList table_exclude_oids = {NULL, NULL};
static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
+static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
+static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
char g_opaque_type[10]; /* name for the opaque type */
@@ -156,6 +158,9 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_foreign_server_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
@@ -388,6 +393,7 @@ main(int argc, char **argv)
{"no-sync", no_argument, NULL, 7},
{"on-conflict-do-nothing", no_argument, _nothing, 1},
{"rows-per-insert", required_argument, NULL, 10},
+ {"include-foreign-data", required_argument, NULL, 11},
{NULL, 0, NULL, 0}
};
@@ -604,6 +610,15 @@ main(int argc, char **argv)
dopt.dump_inserts = (int) rowsPerInsert;
break;
+ case 11:/* include foreign data */
+if (strcmp(optarg, "") == 0)
+{
+ pg_log_error("empty string is not a valid pattern in --include-foreign-data");
+ exit_nicely(1);
+}
+simple_string_list_append(_servers_include_patterns, optarg);
+break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -645,6 +660,12 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (dopt.schemaOnly && foreign_servers_include_patterns.head != NULL)
+ {
+ pg_log_error("options -s/--schema-only and --include-foreign-data cannot be used together");
+ exit_nicely(1);
+ }
+
if (dopt.dataOnly && dopt.outputClean)
{
pg_log_error("options -c/--clean and -a/--data-only cannot be used together");
@@ -812,6 +833,9 @@ main(int argc, char **argv)
_exclude_oids,
false);
+ expand_foreign_server_name_patterns(fout, _servers_include_patterns,
+ _servers_include_oids);
+
/* non-matching exclusion patterns aren't an error */
/*
@@ -1035,6 +1059,9 @@ help(const char *progname)
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
+ printf(_(" --include-foreign-data=PATTERN\n"
+ " include data of foreign tables with the named\n"
+ " foreign servers in dump\n"));
printf(_("\nConnection options:\n"));
printf(_(" -d, --dbname=DBNAME database to dump\n"));
@@ -1333,6 +1360,53 @@ expand_schema_name_patterns(Archive *fout,
destroyPQExpBuffer(query);
}
+/*
+ * Find the OIDs of all foreign servers matching the given list of patterns,
+ * and append them to the given OID list.
+ */
+static void
Re: Option to dump foreign data in pg_dump
Hello
a new version of the patch with the tests from Daniel (thanks!) and the
nitpicks.
I don't feel good about this feature.
pg_dump should not dump any data that are not part of the database
being dumped.
If you restore such a dump, the data will be inserted into the foreign table,
right? Unless someone emptied the remote table first, this will add
duplicated data to that table.
I think that is an unpleasant surprise. I'd expect that if I drop a database
and restore it from a dump, it should be as it was before. This change would
break that assumption.
What are the use cases of a dump with foreign table data?
Unless I misunderstood something there, -1.
This feature is opt-in so if the user makes dumps of a remote server explicitly
by other means, then the user would not need to use these option.
But, not all foreign tables are necessarily in a remote server like the ones
referenced by the postgres_fdw.
In FDWs like swarm64da, cstore, citus or timescaledb, the foreign tables are
part of your database, and one could expect that a dump of the database
includes data from these FDWs.
Cheers
Luis M Carril
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 4bcd4bdaef..319851b936 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -767,6 +767,34 @@ PostgreSQL documentation
+
+ --include-foreign-data=foreignserver
+
+
+Dump the data for any foreign table with a foreign server
+matching foreignserver
+pattern. Multiple foreign servers can be selected by writing multiple --include-foreign-data.
+Also, the foreignserver parameter is
+interpreted as a pattern according to the same rules used by
+psql's \d commands (see ),
+so multiple foreign servers can also be selected by writing wildcard characters
+in the pattern. When using wildcards, be careful to quote the pattern
+if needed to prevent the shell from expanding the wildcards; see
+.
+The only exception is that an empty pattern is disallowed.
+
+
+
+
+ When --include-foreign-data is specified, pg_dump
+ does not check if the foreign table is also writeable. Therefore, there is no guarantee that
+ the results of a foreign table dump can be successfully restored by themselves into a clean database.
+
+
+
+
+
--inserts
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bf69adc2f4..31465dec79 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -120,6 +120,8 @@ static SimpleStringList table_exclude_patterns = {NULL, NULL};
static SimpleOidList table_exclude_oids = {NULL, NULL};
static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
+static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
+static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
char g_opaque_type[10]; /* name for the opaque type */
@@ -156,6 +158,9 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_foreign_server_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
@@ -388,6 +393,7 @@ main(int argc, char **argv)
{"no-sync", no_argument, NULL, 7},
{"on-conflict-do-nothing", no_argument, _nothing, 1},
{"rows-per-insert", required_argument, NULL, 10},
+ {"include-foreign-data", required_argument, NULL, 11},
{NULL, 0, NULL, 0}
};
@@ -604,6 +610,15 @@ main(int argc, char **argv)
dopt.dump_inserts = (int) rowsPerInsert;
break;
+ case 11:/* include foreign data */
+if (strcmp(optarg, "") == 0)
+{
+ pg_log_error("empty string is not a valid pattern in --include-foreign-data");
+ exit_nicely(1);
+}
+simple_string_list_append(_servers_include_patterns, optarg);
+break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -645,6 +660,12 @@ main(int argc, char **argv)
exit_nicely(1);
}
+ if (dopt.schemaOnly && foreign_servers_include_patterns.head != NULL)
+ {
+ pg_log_error("options -s/--schema-only and --include-foreign-data cannot be used together");
+ exit_nicely(1);
+ }
+
if (dopt.dataOnly && dopt.outputClean)
{
pg_log_error("options -c/--clean and -a/--data-only cannot be used together");
@@ -812,6 +833,9 @@ main(int argc, char **argv)
_exclude_oids,
false);
+ expand_foreign_server_name_patterns(fout, _servers_include_patterns,
+ _servers_include_oids);
+
/* non-matching
Re: Add FOREIGN to ALTER TABLE in pg_dump
I don't disagree with adding FOREIGN, though.
Your patch is failing the pg_dump TAP tests. Please use
configure --enable-tap-tests, fix the problems, then resubmit.
Fixed, I've attached a new version.
Cheers
Luis M Carril
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f01fea5b91..65abc22e7a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15550,6 +15550,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
{
char *ftoptions = NULL;
char *srvname = NULL;
+ char *foreign = "";
switch (tbinfo->relkind)
{
@@ -15583,6 +15584,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
ftoptions = pg_strdup(PQgetvalue(res, 0, i_ftoptions));
PQclear(res);
destroyPQExpBuffer(query);
+
+ foreign = "FOREIGN ";
break;
}
case RELKIND_MATVIEW:
@@ -15924,7 +15927,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
continue;
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inherited constraint.\n");
-appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, " ADD CONSTRAINT %s ",
fmtId(constr->dobj.name));
@@ -15945,7 +15948,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
{
TableInfo *parentRel = parents[k];
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT %s;\n",
+ appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s INHERIT %s;\n", foreign,
qualrelname,
fmtQualifiedDumpable(parentRel));
}
@@ -16051,7 +16054,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (!shouldPrintColumn(dopt, tbinfo, j) &&
tbinfo->notnull[j] && !tbinfo->inhNotNull[j])
{
-appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s SET NOT NULL;\n",
fmtId(tbinfo->attnames[j]));
@@ -16064,7 +16067,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
*/
if (tbinfo->attstattarget[j] >= 0)
{
-appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s ",
fmtId(tbinfo->attnames[j]));
@@ -16101,7 +16104,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
*/
if (storage != NULL)
{
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+ appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s ",
fmtId(tbinfo->attnames[j]));
@@ -16115,7 +16118,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
*/
if (tbinfo->attoptions[j][0] != '\0')
{
-appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s ",
fmtId(tbinfo->attnames[j]));
@@ -16238,6 +16241,7 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
PQExpBuffer delq;
char *qualrelname;
char *tag;
+ char *foreign;
/* Skip if table definition not to be dumped */
if (!tbinfo->dobj.dump || dopt->dataOnly)
@@ -16252,13 +16256,15 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
qualrelname = pg_strdup(fmtQualifiedDumpable(tbinfo));
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+ foreign = tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "";
+
+ appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s SET DEFAULT %s;\n",
fmtId(tbinfo->attnames[adnum - 1]),
adinfo->adef_expr);
- appendPQExpBuffer(delq, "ALTER TABLE %s ",
+ appendPQExpBuffer(delq, "ALTER %sTABLE %s ", foreign,
qualrelname);
appendPQExpBuffer(delq, "ALTER COLUMN %s DROP DEFAULT;\n",
fmtId(tbinfo->attnames[adnum - 1]));
@@ -16564,6 +16570,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
PQExpBuffer q;
PQExpBuffer delq;
char *tag = NULL;
+ char *foreign;
/* Skip if not to be dumped */
if (!coninfo->dobj.dump || dopt->dataOnly)
@@ -16572,6 +16579,8 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
q = createPQExpBuffer();
delq = createPQExpBuffer();
+ foreign = tbinfo && tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN " : "";
+
if (coninfo->contype == 'p' ||
coninfo->contype == 'u' ||
coninfo->contype == 'x')
@@ -16590,7 +16599,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
binary_upgrade_set_pg_class_oids(fout, q,
indxinfo->dobj.catId.oid, true);
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s\n",
+ appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s\n", foreign,
fmtQualifiedDumpable(tbinfo));
appendPQExpBuffer(q, "ADD CONSTRAINT %s ",
fmtId(coninfo->dobj.name));
@@ -16680,7 +16689,7 @@
Re: Option to dump foreign data in pg_dump
On Fri, Sep 20, 2019 at 6:20 PM Luis Carril mailto:luis.car...@swarm64.com>> wrote: Hello, thanks for the comments! * + if (tdinfo->filtercond || tbinfo->relkind == RELKIND_FOREIGN_TABLE) filter condition is not implemented completely yet so the logic only work on foreign table so I think its better to handle it separately Note that there is another if condition that actually applies the the filtercondition if provided, also for a we need to do a COPY SELECT instead of a COPY TO but we can't supplied where clause in pg_dump yet so filtercondtion is always NULL and the logic became true only on foreign table. * I don’t understand the need for changing SELECT query .we can use the same SELECT query syntax for both regular table and foreign table To which query do you refer? In the patch there are three queries: 1 retrieves foreign servers, another is the SELECT in the COPY that now it applies in case of a filter condition of a foreign table, and a third that retrieves the oid of a given foreign server. SELECT on COPY regards Surafel If we have a non-foreign table and filtercond is NULL, then we can do a `COPY table columns TO stdout`. But if the table is foreign, the `COPY foreign-table columns TO stdout` is not supported by Postgres, so we have to do a `COPY (SELECT columns FROM foreign-table) TO sdout` Now if in any case the filtercond is non-NULL, ie we have a WHERE clause, then for non-foreign and foreign tables we have to do a: `COPY (SELECT columns FROM table) TO sdout` So the COPY of a foreign table has to be done using the sub-SELECT just as a non-foreign table with filtercond, not like a non-foreign table without filtercond. Cheers Luis M Carril
Re: Option to dump foreign data in pg_dump
On 15.07.19 12:06, Daniel Gustafsson wrote:
On 12 Jul 2019, at 16:08, Luis Carril wrote:
On 28 Jun 2019, at 19:55, Luis Carril wrote:
What about providing a list of FDW servers instead of an all or nothing option?
In that way the user really has to do a conscious decision to dump the content
of the foreign tables for > > a specific server, this would allow distinction
if multiple FDW are being used in the same DB.
I think this is a good option, the normal exclusion rules can then still apply
in case not everything from a specific server is of interest.
Hi, here is a new patch to dump the data of foreign tables using pg_dump.
Cool! Please register this patch in the next commitfest to make sure it
doesn’t get lost on the way. Feel free to mark me as reviewer when adding it.
Thanks, I'll do!
This time the user specifies for which foreign servers the data will be dumped,
which helps in case of having a mix of writeable and non-writeable fdw in the
database.
Looks good, and works as expected.
A few comments on the patch:
Documentation is missing, but you've waited with docs until the functionality
of the patch was fleshed out?
I've added the documentation about the option in the pg_dump page
This allows for adding a blanket wildcard with "--include-foreign-data=“ which
includes every foreign server. This seems to go against the gist of the patch,
to require an explicit opt-in per server. Testing for an empty string should
do the trick.
+ case 11:/* include foreign data */
+ simple_string_list_append(_servers_include_patterns,
optarg);
+ break;
+
Now it errors if any is an empty string.
I don’t think expand_foreign_server_name_patterns should use strict_names, but
rather always consider failures to map as errors.
+ expand_foreign_server_name_patterns(fout,
_servers_include_patterns,
+ _servers_include_oids,
+ strict_names);
Removed, ie if nothing match it throws an error.
This seems like a bit too ambiguous name, it would be good to indicate in the
name that it refers to a foreign server.
+ Oid serveroid; /* foreign server oid */
Changed to foreign_server_oid.
As coded there is no warning when asking for foreign data on a schema-only
dump, maybe something like could make usage clearer as this option is similar
in concept to data-only:
+ if (dopt.schemaOnly && foreign_servers_include_patterns.head != NULL)
+ {
+ pg_log_error("options -s/--schema-only and --include-foreign-data
cannot be used together");
+ exit_nicely(1);
+ }
+
Added too
cheers ./daniel
Thanks for the comments!
Cheers
Luis M Carril
From 7bab80b983cf3ce9e8ecdf7d1a9113d08347e047 Mon Sep 17 00:00:00 2001
From: Luis Carril
Date: Fri, 28 Jun 2019 16:05:43 +0200
Subject: [PATCH] Support foreign data in pg_dump
---
doc/src/sgml/ref/pg_dump.sgml | 28 +
src/bin/pg_dump/pg_dump.c | 115 --
src/bin/pg_dump/pg_dump.h | 1 +
3 files changed, 138 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 8fa2314347..db52abe39b 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -767,6 +767,34 @@ PostgreSQL documentation
+
+ --include-foreign-data=foreignserver
+
+
+Dump the data for any foreign table with a foreign server
+matching foreignserver
+pattern. Multiple foreign servers can be selected by writing multiple --include-foreign-data.
+Also, the foreignserver parameter is
+interpreted as a pattern according to the same rules used by
+psql's \d commands (see ),
+so multiple foreign servers can also be selected by writing wildcard characters
+in the pattern. When using wildcards, be careful to quote the pattern
+if needed to prevent the shell from expanding the wildcards; see
+.
+The only exception is that an empty pattern is disallowed.
+
+
+
+
+ When --include-foreign-data is specified, pg_dump
+ does not check if the foreign table is also writeable. Therefore, there is no guarantee that
+ the results of a foreign table dump can be successfully restored by themselves into a clean database.
+
+
+
+
+
--inserts
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 806fc78f04..b1c21eede5 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -123,6 +123,8 @@ static SimpleStringList table_exclude_patterns = {NULL, NULL};
static SimpleOidList table_exclude_oids = {NULL, NULL};
static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleO
Re: Option to dump foreign data in pg_dump
> > On 28 Jun 2019, at 19:55, Luis Carril wrote:
> > What about providing a list of FDW servers instead of an all or nothing
> > option? In that way the user really has to do a conscious decision to dump
> > the content of the foreign tables for > > a specific server, this would
> > allow distinction if multiple FDW are being used in the same DB.
> I think this is a good option, the normal exclusion rules can then still apply
> in case not everything from a specific server is of interest.
Hi, here is a new patch to dump the data of foreign tables using pg_dump.
This time the user specifies for which foreign servers the data will be dumped,
which helps in case of having a mix of writeable and non-writeable fdw in the
database.
It would be nice to emit an error if the fdw is read-only, but that information
is not available in the catalog.
Cheers
Luis M Carril
From d24cbf4ad0852b079d0e16103486873ab6bb8b69 Mon Sep 17 00:00:00 2001
From: Luis Carril
Date: Fri, 28 Jun 2019 16:05:43 +0200
Subject: [PATCH] Support foreign data in pg_dump
---
src/bin/pg_dump/pg_dump.c | 107 +++---
src/bin/pg_dump/pg_dump.h | 1 +
2 files changed, 102 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 806fc78f04..ceff6a1744 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -123,6 +123,8 @@ static SimpleStringList table_exclude_patterns = {NULL, NULL};
static SimpleOidList table_exclude_oids = {NULL, NULL};
static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
+static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
+static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
char g_opaque_type[10]; /* name for the opaque type */
@@ -159,6 +161,10 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_foreign_server_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
@@ -391,6 +397,7 @@ main(int argc, char **argv)
{"no-sync", no_argument, NULL, 7},
{"on-conflict-do-nothing", no_argument, _nothing, 1},
{"rows-per-insert", required_argument, NULL, 10},
+ {"include-foreign-data", required_argument, NULL, 11},
{NULL, 0, NULL, 0}
};
@@ -607,6 +614,10 @@ main(int argc, char **argv)
dopt.dump_inserts = (int) rowsPerInsert;
break;
+ case 11:/* include foreign data */
+simple_string_list_append(_servers_include_patterns, optarg);
+break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -815,6 +826,15 @@ main(int argc, char **argv)
_exclude_oids,
false);
+ if (foreign_servers_include_patterns.head != NULL)
+ {
+ expand_foreign_server_name_patterns(fout, _servers_include_patterns,
+ _servers_include_oids,
+ strict_names);
+ if (foreign_servers_include_oids.head == NULL)
+ fatal("no matching foreign servers were found");
+ }
+
/* non-matching exclusion patterns aren't an error */
/*
@@ -1038,6 +1058,9 @@ help(const char *progname)
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
+ printf(_(" --include-foreign-data=SERVER\n"
+ " include data of foreign tables with the named\n"
+ " foreign servers in dump\n"));
printf(_("\nConnection options:\n"));
printf(_(" -d, --dbname=DBNAME database to dump\n"));
@@ -1336,6 +1359,54 @@ expand_schema_name_patterns(Archive *fout,
destroyPQExpBuffer(query);
}
+/*
+ * Find the OIDs of all foreign servers matching the given list of patterns,
+ * and append them to the given OID list.
+ */
+static void
+expand_foreign_server_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names)
+{
+ PQExpBuffer query;
+ PGresult *res;
+ SimpleStringListCell *cell;
+ int i;
+
+ if (patterns->head == NULL)
+ return; /* nothing to do */
+
+ query = createPQExpBuffer();
+
+ /*
+ * The loop below runs multiple SELECTs might sometimes result in
+ * duplicate entries in the OID list, but we don't care.
+ */
+
+ for (cell = patterns->head; cell; cell = cell->next)
+ {
+ appendPQExpBuf
Add FOREIGN to ALTER TABLE in pg_dump
Hello,
pg_dump creates plain ALTER TABLE statements even if the table is a foreign
table, which for someone reading the dump is confusing.
This also made a difference when applying the dump if there is any plugin
installed that hooks on ProcessUtility, because the plugin could react
differently to ALTER TABLE than to ALTER FOREIGN TABLE. Opinions?
An unrelated question: if I apply pgindent to a file (in this case
pg_dump.c) and get a bunch of changes on the indentation that are not related
to my patch, which is the accepted policy? A different patch first with only
the indentation? Maybe, am I using pgindent wrong?
Cheers
Luis M Carril
From a5b439b7dfdc1f9bab8d2cf1fc95a8eaedb1d83e Mon Sep 17 00:00:00 2001
From: Luis Carril
Date: Fri, 12 Jul 2019 12:51:17 +0200
Subject: [PATCH] Add FOREIGN to ALTER statements
---
src/bin/pg_dump/pg_dump.c | 40 ---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 806fc78f04..72951f9a0a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15535,6 +15535,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
{
char *ftoptions = NULL;
char *srvname = NULL;
+ char *foreign = "";
switch (tbinfo->relkind)
{
@@ -15568,6 +15569,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
ftoptions = pg_strdup(PQgetvalue(res, 0, i_ftoptions));
PQclear(res);
destroyPQExpBuffer(query);
+
+ foreign = "FOREIGN";
break;
}
case RELKIND_MATVIEW:
@@ -15909,7 +15912,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
continue;
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inherited constraint.\n");
-appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+appendPQExpBuffer(q, "ALTER %s TABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, " ADD CONSTRAINT %s ",
fmtId(constr->dobj.name));
@@ -15930,7 +15933,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
{
TableInfo *parentRel = parents[k];
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT %s;\n",
+ appendPQExpBuffer(q, "ALTER %s TABLE ONLY %s INHERIT %s;\n", foreign,
qualrelname,
fmtQualifiedDumpable(parentRel));
}
@@ -16036,7 +16039,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (!shouldPrintColumn(dopt, tbinfo, j) &&
tbinfo->notnull[j] && !tbinfo->inhNotNull[j])
{
-appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+appendPQExpBuffer(q, "ALTER %s TABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s SET NOT NULL;\n",
fmtId(tbinfo->attnames[j]));
@@ -16049,7 +16052,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
*/
if (tbinfo->attstattarget[j] >= 0)
{
-appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+appendPQExpBuffer(q, "ALTER %s TABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s ",
fmtId(tbinfo->attnames[j]));
@@ -16086,7 +16089,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
*/
if (storage != NULL)
{
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+ appendPQExpBuffer(q, "ALTER %s TABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s ",
fmtId(tbinfo->attnames[j]));
@@ -16100,7 +16103,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
*/
if (tbinfo->attoptions[j][0] != '\0')
{
-appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+appendPQExpBuffer(q, "ALTER %s TABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s ",
fmtId(tbinfo->attnames[j]));
@@ -16223,6 +16226,7 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
PQExpBuffer delq;
char *qualrelname;
char *tag;
+ char *foreign;
/* Skip if table definition not to be dumped */
if (!tbinfo->dobj.dump || dopt->dataOnly)
@@ -16237,13 +16241,15 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
qualrelname = pg_strdup(fmtQualifiedDumpable(tbinfo));
- appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+ foreign = tbinfo->relkind == RELKIND_FOREIGN_TABLE ? "FOREIGN" : "";
+
+ appendPQExpBuffer(q, "ALTER %s TABLE ONLY %s ", foreign,
qualrelname);
appendPQExpBuffer(q, "ALTER COLUMN %s SET DEFAULT %s;\n",
fmtId(tbinfo->attnames[adnum - 1]),
adinfo->adef_expr);
- appendPQExpBuffer(delq, "ALTER TABLE %s ",
+ appendPQExpBuffer(delq, "ALTER %s TABLE %s ", foreign,
qualrelname);
appe
Re: Option to dump foreign data in pg_dump
>Restoring content of FDW table via pg_restore or psql can be dangerous - >there I see a risk, and can be nice to allow it only >with some form of >safeguard. > >I think so important questions is motivation for dumping FDW - a) clonning >(has sense for me and it is safe), b) real backup >(requires writeable FDW) - >has sense too, but I see a possibility of unwanted problems. What about providing a list of FDW servers instead of an all or nothing option? In that way the user really has to do a conscious decision to dump the content of the foreign tables for a specific server, this would allow distinction if multiple FDW are being used in the same DB. Also I think it is responsability of the user to know if the FDW that are being used are read-only or not. Cheers Luis M Carril
Option to dump foreign data in pg_dump
Hello,
pg_dump ignores the dumping of data in foreign tables
on purpose, this patch makes it optional as the user maybe
wants to manage the data in the foreign servers directly from
Postgres. Opinions?
Cheers
Luis M Carril
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index db30b54a92..a21027a61d 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -159,6 +159,7 @@ typedef struct _dumpOptions
int use_setsessauth;
int enable_row_security;
int load_via_partition_root;
+ int include_foreign_data;
/* default, if no "inclusion" switches appear, is to dump everything */
bool include_everything;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8909a45d61..8854f70305 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -391,6 +391,7 @@ main(int argc, char **argv)
{"no-sync", no_argument, NULL, 7},
{"on-conflict-do-nothing", no_argument, _nothing, 1},
{"rows-per-insert", required_argument, NULL, 10},
+ {"include-foreign-data", no_argument, _foreign_data, 1},
{NULL, 0, NULL, 0}
};
@@ -1038,6 +1039,7 @@ help(const char *progname)
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
+ printf(_(" --include-foreign-data include data of foreign tables in dump\n"));
printf(_("\nConnection options:\n"));
printf(_(" -d, --dbname=DBNAME database to dump\n"));
@@ -1812,7 +1814,7 @@ dumpTableData_copy(Archive *fout, void *dcontext)
*/
column_list = fmtCopyColumnList(tbinfo, clistBuf);
- if (tdinfo->filtercond)
+ if (tdinfo->filtercond || tbinfo->relkind == RELKIND_FOREIGN_TABLE)
{
/* Note: this syntax is only supported in 8.2 and up */
appendPQExpBufferStr(q, "COPY (SELECT ");
@@ -1824,9 +1826,11 @@ dumpTableData_copy(Archive *fout, void *dcontext)
}
else
appendPQExpBufferStr(q, "* ");
- appendPQExpBuffer(q, "FROM %s %s) TO stdout;",
- fmtQualifiedDumpable(tbinfo),
- tdinfo->filtercond);
+
+ appendPQExpBuffer(q, "FROM %s", fmtQualifiedDumpable(tbinfo));
+ if (tdinfo->filtercond)
+ appendPQExpBuffer(q, " %s", tdinfo->filtercond);
+ appendPQExpBuffer(q, ") TO stdout;");
}
else
{
@@ -2343,7 +2347,7 @@ makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo)
if (tbinfo->relkind == RELKIND_VIEW)
return;
/* Skip FOREIGN TABLEs (no data to dump) */
- if (tbinfo->relkind == RELKIND_FOREIGN_TABLE)
+ if (tbinfo->relkind == RELKIND_FOREIGN_TABLE && !dopt->include_foreign_data)
return;
/* Skip partitioned tables (data in partitions) */
if (tbinfo->relkind == RELKIND_PARTITIONED_TABLE)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 24719cefe2..14d62e9781 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -82,6 +82,7 @@ static int no_role_passwords = 0;
static int server_version;
static int load_via_partition_root = 0;
static int on_conflict_do_nothing = 0;
+static int include_foreign_data = 0;
static char role_catalog[10];
#define PG_AUTHID "pg_authid"
@@ -147,6 +148,7 @@ main(int argc, char *argv[])
{"no-unlogged-table-data", no_argument, _unlogged_table_data, 1},
{"on-conflict-do-nothing", no_argument, _conflict_do_nothing, 1},
{"rows-per-insert", required_argument, NULL, 7},
+ {"include-foreign-data", no_argument, _foreign_data, 1},
{NULL, 0, NULL, 0}
};
@@ -432,6 +434,8 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --no-unlogged-table-data");
if (on_conflict_do_nothing)
appendPQExpBufferStr(pgdumpopts, " --on-conflict-do-nothing");
+ if (include_foreign_data)
+ appendPQExpBufferStr(pgdumpopts, " --include-foreign-data");
/*
* If there was a database specified on the command line, use that,
@@ -658,6 +662,7 @@ help(void)
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
+ printf(_(" --include-foreign-data include data of foreign tables in dump\n"));
printf(_("\nConnection options:\n"));
printf(_(" -d, --dbname=CONNSTR connect using connection string\n"));
