> 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.
> 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?
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;
+
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);
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 */
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);
+ }
+
cheers ./daniel
