2015-03-14 19:33 GMT+01:00 Pavel Stehule <pavel.steh...@gmail.com>:
>
>
> 2015-03-13 23:43 GMT+01:00 Josh Berkus <j...@agliodbs.com>:
>
>> On 03/13/2015 10:01 AM, Pavel Stehule wrote:
>> >
>> >
>> > 2015-03-13 17:39 GMT+01:00 Robert Haas <robertmh...@gmail.com
>> > <mailto:robertmh...@gmail.com>>:
>> >
>> > On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
>> > <pavel.steh...@gmail.com <mailto:pavel.steh...@gmail.com>> 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 <pavel.steh...@gooddata.com>
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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
