Re: Option to dump foreign data in pg_dump

On Mon, Jul 15, 2019 at 6:09 PM Luis Carril <luis(dot)carril(at)swarm64(dot)com> wrote:
>
> On 15.07.19 12:06, Daniel Gustafsson wrote:
>
Few comments:

As you have specified required_argument in above:
+ {"include-foreign-data", required_argument, NULL, 11},

The below check may not be required:
+ if (strcmp(optarg, "") == 0)
+ {
+ pg_log_error("empty string is not a valid pattern in --include-foreign-data");
+ exit_nicely(1);
+ }

+ 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");
+ }
+

The above check if (foreign_servers_include_oids.head == NULL) may not
be required, as there is a check present inside
expand_foreign_server_name_patterns to handle this error:
+
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (PQntuples(res) == 0)
+ fatal("no matching foreign servers were found for pattern \"%s\"", cell->val);
+

+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 */
+

The above check for patterns->head may not be required as similar
check exists before this function is called:
+ 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");
+ }
+

+ /* 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)))
simple_oid_list_member can be split into two lines

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com