2015-03-14 19:33 GMT+01:00 Pavel Stehule <[email protected]>: > > > 2015-03-13 23:43 GMT+01:00 Josh Berkus <[email protected]>: > >> On 03/13/2015 10:01 AM, Pavel Stehule wrote: >> > >> > >> > 2015-03-13 17:39 GMT+01:00 Robert Haas <[email protected] >> > <mailto:[email protected]>>: >> > >> > On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule >> > <[email protected] <mailto:[email protected]>> wrote: >> > > we found possible bug in pg_dump. It raise a error only when all >> > specified >> > > tables doesn't exists. When it find any table, then ignore missing >> > other. >> > > >> > > /usr/local/pgsql/bin/pg_dump -t Foo -t omega -s postgres > >> > /dev/null; echo >> > > $? >> > > >> > > foo doesn't exists - it creates broken backup due missing "Foo" >> table >> > > >> > > [pavel@localhost include]$ /usr/local/pgsql/bin/pg_dump -t Foo >> -t >> > omegaa -s >> > > postgres > /dev/null; echo $? >> > > pg_dump: No matching tables were found >> > > 1 >> > > >> > > Is it ok? I am thinking, so it is potentially dangerous. Any >> > explicitly >> > > specified table should to exists. >> > >> > Keep in mind that the argument to -t is a pattern, not just a table >> > name. I'm not sure how much that affects the calculus here, but >> it's >> > something to think about. >> > >> > >> > yes, it has a sense, although now, I am don't think so it was a good >> > idea. There should be some difference between table name and table >> pattern. >> >> There was a long discussion about this when the feature was introduced >> in 7.3(?) IIRC. Changing it now would break backwards-compatibility, so >> you'd really need to introduce a new option. >> >> And, if you introduce a new option which is a specific table name, would >> you require a schema name or not? >> > > We can use a same rules like we use for STRICT clause somewhere. There > should be only one table specified by the option. If it is not necessary, > then only name is enough, else qualified name is necessary. > > what is a name for this option? Maybe only with long name - some like > 'required-table' ? >
other variant, I hope better than previous. We can introduce new long option "--strict". With this active option, every pattern specified by -t option have to have identifies exactly only one table. It can be used for any other "should to exists" patterns - schemas. Initial implementation in attachment. Regards Pavel > > Regards > > Pavel > > >> >> -- >> Josh Berkus >> PostgreSQL Experts Inc. >> http://pgexperts.com >> > >
commit 3d3c6f6583c44a06633aea7978ad0631fed1679b Author: Pavel Stehule <[email protected]> Date: Sun Mar 15 00:53:49 2015 +0100 initial diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index fdfb431..2a0d50f 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -134,7 +134,8 @@ static void expand_schema_name_patterns(Archive *fout, SimpleOidList *oids); static void expand_table_name_patterns(Archive *fout, SimpleStringList *patterns, - SimpleOidList *oids); + SimpleOidList *oids, + bool strict_table_names); static NamespaceInfo *findNamespace(Archive *fout, Oid nsoid, Oid objoid); static void dumpTableData(Archive *fout, DumpOptions *dopt, TableDataInfo *tdinfo); static void refreshMatViewData(Archive *fout, TableDataInfo *tdinfo); @@ -253,6 +254,8 @@ static char *get_synchronized_snapshot(Archive *fout); static PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query); static void setupDumpWorker(Archive *AHX, DumpOptions *dopt, RestoreOptions *ropt); +static int strict_table_names = false; + int main(int argc, char **argv) @@ -332,6 +335,7 @@ main(int argc, char **argv) {"section", required_argument, NULL, 5}, {"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1}, {"snapshot", required_argument, NULL, 6}, + {"strict", no_argument, &strict_table_names, 1}, {"use-set-session-authorization", no_argument, &dopt.use_setsessauth, 1}, {"no-security-labels", no_argument, &dopt.no_security_labels, 1}, {"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1}, @@ -700,15 +704,18 @@ main(int argc, char **argv) if (table_include_patterns.head != NULL) { expand_table_name_patterns(fout, &table_include_patterns, - &table_include_oids); + &table_include_oids, + strict_table_names); if (table_include_oids.head == NULL) exit_horribly(NULL, "No matching tables were found\n"); } expand_table_name_patterns(fout, &table_exclude_patterns, - &table_exclude_oids); + &table_exclude_oids, + false); expand_table_name_patterns(fout, &tabledata_exclude_patterns, - &tabledata_exclude_oids); + &tabledata_exclude_oids, + false); /* non-matching exclusion patterns aren't an error */ @@ -1173,13 +1180,30 @@ expand_schema_name_patterns(Archive *fout, destroyPQExpBuffer(query); } +static void +append_table_name_query(Archive *fout, PQExpBuffer query, char *tablename) +{ + appendPQExpBuffer(query, + "SELECT c.oid" + "\nFROM pg_catalog.pg_class c" + "\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace" + "\nWHERE c.relkind in ('%c', '%c', '%c', '%c', '%c')\n", + RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW, + RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE); + + processSQLNamePattern(GetConnection(fout), query, tablename, true, + false, "n.nspname", "c.relname", NULL, + "pg_catalog.pg_table_is_visible(c.oid)"); +} + /* * Find the OIDs of all tables matching the given list of patterns, * and append them to the given OID list. */ static void expand_table_name_patterns(Archive *fout, - SimpleStringList *patterns, SimpleOidList *oids) + SimpleStringList *patterns, SimpleOidList *oids, + bool strict_table_names) { PQExpBuffer query; PGresult *res; @@ -1196,31 +1220,43 @@ expand_table_name_patterns(Archive *fout, * duplicate entries in the OID list, but we don't care. */ - for (cell = patterns->head; cell; cell = cell->next) + if (!strict_table_names) { - if (cell != patterns->head) - appendPQExpBufferStr(query, "UNION ALL\n"); - appendPQExpBuffer(query, - "SELECT c.oid" - "\nFROM pg_catalog.pg_class c" - "\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace" - "\nWHERE c.relkind in ('%c', '%c', '%c', '%c', '%c')\n", - RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW, - RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE); - processSQLNamePattern(GetConnection(fout), query, cell->val, true, - false, "n.nspname", "c.relname", NULL, - "pg_catalog.pg_table_is_visible(c.oid)"); - } + for (cell = patterns->head; cell; cell = cell->next) + { + if (cell != patterns->head) + appendPQExpBufferStr(query, "UNION ALL\n"); + append_table_name_query(fout, query, cell->val); + } - res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); + res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); + for (i = 0; i < PQntuples(res); i++) + { + simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0))); + } - for (i = 0; i < PQntuples(res); i++) - { - simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0))); + PQclear(res); + destroyPQExpBuffer(query); } + else + { + for (cell = patterns->head; cell; cell = cell->next) + { + query = createPQExpBuffer(); + append_table_name_query(fout, query, cell->val); - PQclear(res); - destroyPQExpBuffer(query); + res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); + if (PQntuples(res) < 1) + exit_horribly(NULL, "No matching table were found for '%s'\n", cell->val); + else if (PQntuples(res) > 1) + exit_horribly(NULL, "More than one table found for '%s'\n", cell->val); + else + simple_oid_list_append(oids, atooid(PQgetvalue(res, 0, 0))); + + PQclear(res); + destroyPQExpBuffer(query); + } + } } /*
-- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
