From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Luis Carril <luis(dot)carril(at)swarm64(dot)com> |
Cc: | Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Option to dump foreign data in pg_dump |
Date: | 2019-09-19 09:48:56 |
Message-ID: | CALDaNm1fV88s9+2nGtXwfCbraKxog_TbJ3Kw9XWyBB+_SSRqjQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 19, 2019 at 3:08 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> 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
>
Also can we include few tests for this feature.
Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Dilip Kumar | 2019-09-19 10:24:05 | Re: pgbench - allow to create partitioned tables |
Previous Message | Kyotaro Horiguchi | 2019-09-19 09:47:08 | Re: Psql patch to show access methods info |