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
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--include-foreign-data=<replaceable class="parameter">foreignserver</replaceable></option></term>
+ <listitem>
+ <para>
+ Dump the data for any foreign table with a foreign server
+ matching <replaceable class="parameter">foreignserver</replaceable>
+ pattern. Multiple foreign servers can be selected by writing multiple <option>--include-foreign-data</option>.
+ Also, the <replaceable class="parameter">foreignserver</replaceable> parameter is
+ interpreted as a pattern according to the same rules used by
+ <application>psql</application>'s <literal>\d</literal> commands (see <xref
+ linkend="app-psql-patterns" endterm="app-psql-patterns-title"/>),
+ 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
+ <xref linkend="pg-dump-examples" endterm="pg-dump-examples-title"/>.
+ The only exception is that an empty pattern is disallowed.
+ </para>
+
+ <note>
+ <para>
+ When <option>--include-foreign-data</option> is specified, <application>pg_dump</application>
+ 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.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--inserts</option></term>
<listitem>
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, &dopt.do_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(&foreign_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)
&tabledata_exclude_oids,
false);
+ expand_foreign_server_name_patterns(fout, &foreign_servers_include_patterns,
+ &foreign_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
+expand_foreign_server_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids)
+{
+ 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)
+ {
+ appendPQExpBuffer(query,
+ "SELECT oid FROM pg_catalog.pg_foreign_server s\n");
+ processSQLNamePattern(GetConnection(fout), query, cell->val, false,
+ false, NULL, "s.srvname", NULL, NULL);
+
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (PQntuples(res) == 0)
+ fatal("no matching foreign servers were found for pattern \"%s\"", cell->val);
+
+ for (i = 0; i < PQntuples(res); i++)
+ {
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
+ }
+
+ destroyPQExpBuffer(query);
+}
+
/*
* Find the OIDs of all tables matching the given list of patterns,
* and append them to the given OID list. See also expand_dbname_patterns()
@@ -1809,7 +1883,11 @@ dumpTableData_copy(Archive *fout, void *dcontext)
*/
column_list = fmtCopyColumnList(tbinfo, clistBuf);
- if (tdinfo->filtercond)
+ /*
+ * COPY TO does not support foreign tables directly, instead we do a
+ * COPY (SELECT ...) TO.
+ */
+ if (tdinfo->filtercond || tbinfo->relkind == RELKIND_FOREIGN_TABLE)
{
/* Note: this syntax is only supported in 8.2 and up */
appendPQExpBufferStr(q, "COPY (SELECT ");
@@ -1821,9 +1899,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
{
@@ -2339,8 +2419,11 @@ makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo)
/* Skip VIEWs (no data to dump) */
if (tbinfo->relkind == RELKIND_VIEW)
return;
- /* Skip FOREIGN TABLEs (no data to dump) */
- if (tbinfo->relkind == RELKIND_FOREIGN_TABLE)
+ /* Skip FOREIGN TABLEs (no data to dump) if not requested explicitly */
+ if (tbinfo->relkind == RELKIND_FOREIGN_TABLE &&
+ (foreign_servers_include_oids.head == NULL ||
+ !simple_oid_list_member(&foreign_servers_include_oids,
+ tbinfo->foreign_server_oid)))
return;
/* Skip partitioned tables (data in partitions) */
if (tbinfo->relkind == RELKIND_PARTITIONED_TABLE)
@@ -6648,6 +6731,26 @@ getTables(Archive *fout, int *numTables)
tblinfo[i].ispartition = (strcmp(PQgetvalue(res, i, i_ispartition), "t") == 0);
tblinfo[i].partbound = pg_strdup(PQgetvalue(res, i, i_partbound));
+ if (tblinfo[i].relkind == RELKIND_FOREIGN_TABLE)
+ {
+ PQExpBuffer query_server = createPQExpBuffer();
+ PGresult *res_server;
+
+ /* retrieve the oid of the foreign server*/
+ appendPQExpBuffer(query_server,
+ "SELECT fs.oid "
+ "FROM pg_catalog.pg_foreign_table ft "
+ "JOIN pg_catalog.pg_foreign_server fs "
+ "ON (fs.oid = ft.ftserver) "
+ "WHERE ft.ftrelid = '%u'",
+ tblinfo[i].dobj.catId.oid);
+
+ res_server = ExecuteSqlQueryForSingleRow(fout, query_server->data);
+ tblinfo[i].foreign_server_oid = atooid(PQgetvalue(res_server, 0, 0));
+ PQclear(res_server);
+ destroyPQExpBuffer(query_server);
+ }
+
/*
* Read-lock target tables to make sure they aren't DROPPED or altered
* in schema before we get around to dumping them.
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 7b2c1524a5..28f78b761b 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -322,6 +322,7 @@ typedef struct _tableInfo
char *partbound; /* partition bound definition */
bool needs_override; /* has GENERATED ALWAYS AS IDENTITY */
char *amname; /* relation access method */
+ Oid foreign_server_oid; /* foreign server oid */
/*
* Stuff computed only for dumpable tables.
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 9ca8a8e608..559b4b3e01 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 74;
+use Test::More tests => 78;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -49,6 +49,18 @@ command_fails_like(
'pg_dump: options -s/--schema-only and -a/--data-only cannot be used together'
);
+command_fails_like(
+ [ 'pg_dump', '-s', '--include-foreign-data', 'xxx' ],
+ qr/\Qpg_dump: error: options -s\/--schema-only and --include-foreign-data cannot be used together\E/,
+ 'pg_dump: options -s/--schema-only and --include-foreign-data cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dump', '--include-foreign-data', '' ],
+ qr/\Qpg_dump: error: empty string is not a valid pattern in --include-foreign-data\E/,
+ 'pg_dump: empty string is not a valid pattern in --include-foreign-data'
+);
+
command_fails_like(
['pg_restore'],
qr{\Qpg_restore: error: one of -d/--dbname and -f/--file must be specified\E},
diff --git a/src/test/modules/test_pg_dump/Makefile b/src/test/modules/test_pg_dump/Makefile
index 6123b994f6..6f95a83b57 100644
--- a/src/test/modules/test_pg_dump/Makefile
+++ b/src/test/modules/test_pg_dump/Makefile
@@ -1,12 +1,12 @@
# src/test/modules/test_pg_dump/Makefile
-MODULE = test_pg_dump
-PGFILEDESC = "test_pg_dump - Test pg_dump with an extension"
+MODULES = test_pg_dump_fdw
+PGFILEDESC = "test_pg_dump - Test pg_dump with extensions"
-EXTENSION = test_pg_dump
-DATA = test_pg_dump--1.0.sql
+EXTENSION = test_pg_dump_fdw test_pg_dump
+DATA = test_pg_dump_fdw--1.0.sql test_pg_dump--1.0.sql
-REGRESS = test_pg_dump
+REGRESS = test_pg_dump test_pg_dump_fdw
TAP_TESTS = 1
ifdef USE_PGXS
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index fb4ecf8aca..6c57d39a56 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -135,6 +135,13 @@ my %pgdump_runs = (
"$tempdir/defaults_tar_format.tar",
],
},
+ include_foreign_data => {
+ dump_cmd => [
+ 'pg_dump',
+ '--include-foreign-data=test_pg_dump_fdw_server',
+ "--file=$tempdir/include_foreign_data.sql",
+ ],
+ },
pg_dumpall_globals => {
dump_cmd => [
'pg_dumpall', '--no-sync',
@@ -220,6 +227,7 @@ my %full_runs = (
createdb => 1,
defaults => 1,
no_privs => 1,
+ include_foreign_data => 1,
no_owner => 1,);
my %tests = (
@@ -537,6 +545,55 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
+ },
+
+ 'CREATE EXTENSION test_pg_dump_fdw' => {
+ create_order => 2,
+ create_sql => 'CREATE EXTENSION test_pg_dump_fdw;',
+ regexp => qr/^
+ \QCREATE EXTENSION IF NOT EXISTS test_pg_dump_fdw WITH SCHEMA public;\E
+ \n/xm,
+ like => {
+ %full_runs,
+ include_foreign_data => 1,
+ schema_only => 1,
+ section_pre_data => 1,
+ },
+ unlike => { binary_upgrade => 1, },
+ },
+
+ 'CREATE SERVER test_pg_dump_fdw_server FOREIGN DATA WRAPPER test_pg_dump_fdw' => {
+ create_order => 3,
+ create_sql => 'CREATE SERVER test_pg_dump_fdw_server FOREIGN DATA WRAPPER test_pg_dump_fdw;',
+ regexp => qr/^
+ \QCREATE SERVER test_pg_dump_fdw_server FOREIGN DATA WRAPPER test_pg_dump_fdw;\E
+ \n/xm,
+ like => {
+ %full_runs,
+ include_foreign_data => 1,
+ schema_only => 1,
+ section_pre_data => 1,
+ },
+ },
+
+ 'include foreign data' => {
+ create_order => 9,
+ create_sql => 'CREATE FOREIGN TABLE t (a INTEGER, b INTEGER) SERVER test_pg_dump_fdw_server;',
+ regexp => qr/^
+ \QCOPY public.t (a, b) FROM stdin;\E\n
+ \Q0 0\E\n
+ \Q1 1\E\n
+ \Q2 2\E\n
+ \Q3 3\E\n
+ \Q4 4\E\n
+ \Q5 5\E\n
+ \Q6 6\E\n
+ \Q7 7\E\n
+ \Q8 8\E\n
+ \Q9 9\E\n
+ \Q10 10\E\n
+ /xm,
+ like => { include_foreign_data => 1, },
},);
#########################################
