On 15.07.19 12:06, Daniel Gustafsson wrote:
On 12 Jul 2019, at 16:08, Luis Carril <luis.car...@swarm64.com> wrote:
On 28 Jun 2019, at 19:55, Luis Carril <luis.car...@swarm64.com> 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(&foreign_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,
&foreign_servers_include_patterns,
+ &foreign_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 <luis.car...@swarm64.com>
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
</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 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 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,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,
@@ -391,6 +396,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}
};
@@ -607,6 +613,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);
@@ -648,6 +663,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");
@@ -815,6 +836,14 @@ main(int argc, char **argv)
&tabledata_exclude_oids,
false);
+ if (foreign_servers_include_patterns.head != NULL)
+ {
+ expand_foreign_server_name_patterns(fout, &foreign_servers_include_patterns,
+ &foreign_servers_include_oids);
+ if (foreign_servers_include_oids.head == NULL)
+ fatal("no matching foreign servers were found");
+ }
+
/* non-matching exclusion patterns aren't an error */
/*
@@ -1038,6 +1067,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 +1368,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()
@@ -1812,7 +1891,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 +1903,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
{
@@ -2342,8 +2423,10 @@ 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)
@@ -6643,6 +6726,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 c3c2ea1473..6c631d0d8f 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -326,6 +326,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.
--
2.20.1
