pg_dump quietly ignore missing tables - is it bug?
Hi
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.
Regards
Pavel
On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule <pavel.stehule@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
1Is 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.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2015-03-13 17:39 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule <pavel.stehule@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
1Is 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.
Regards
Pavel
Show quoted text
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Fri, Mar 13, 2015 at 10:01 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
2015-03-13 17:39 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule <pavel.stehule@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
1Is 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 is...a single table name is simply expressed as a pattern without
any wildcards. The issue here is that pg_dump doesn't require that every
instance of -t find one (or more, if a wildcard is present) entries only
that at least one entry is found among all of the patterns specified by -t.
I'll voice my agreement that each of the -t specifications should find at
least one table in order for the dump as a whole to succeed; though
depending on presented use cases for the current behavior I could see
allowing the command writer to specify a more lenient interpretation by
specifying something like --allow-missing-tables.
Command line switch formats don't really allow you to write "-t?" to mean
"I want these table(s) if present", do they? I guess the input itself
could be interpreted that way though; a leading "?" is not a valid wildcard
and double-quotes would be required for it to be a valid table name.
David J.
On 03/13/2015 10:01 AM, Pavel Stehule wrote:
2015-03-13 17:39 GMT+01:00 Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>>:On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
<pavel.stehule@gmail.com <mailto:pavel.stehule@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
1Is 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?
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: WM33c0f52fb124aeea79a83caae184a3bc43148508f74a9f700c070e615543570f4a18531037a81c68635e039cebe0f9d8@asav-3.01.com
2015-03-13 23:43 GMT+01:00 Josh Berkus <josh@agliodbs.com>:
On 03/13/2015 10:01 AM, Pavel Stehule wrote:
2015-03-13 17:39 GMT+01:00 Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>>:On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
<pavel.stehule@gmail.com <mailto:pavel.stehule@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
1Is 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 tablepattern.
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' ?
Regards
Pavel
Show quoted text
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
2015-03-14 19:33 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
2015-03-13 23:43 GMT+01:00 Josh Berkus <josh@agliodbs.com>:
On 03/13/2015 10:01 AM, Pavel Stehule wrote:
2015-03-13 17:39 GMT+01:00 Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>>:On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
<pavel.stehule@gmail.com <mailto:pavel.stehule@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
1Is 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, butit'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 tablepattern.
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
Show quoted text
Regards
Pavel
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
Attachments:
pg_dump.patchtext/x-patch; charset=US-ASCII; name=pg_dump.patchDownload
commit 3d3c6f6583c44a06633aea7978ad0631fed1679b
Author: Pavel Stehule <pavel.stehule@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);
+ }
+ }
}
/*
Pavel Stehule <pavel.stehule@gmail.com> writes:
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.
I think this design is seriously broken. If I have '-t foo*' the code
should not prevent that from matching multiple tables. What would the use
case for such a restriction be?
What would make sense to me is one or both of these ideas:
* require a match for a wildcard-free -t switch
* require at least one (not "exactly one") match for a wildcarded -t
switch.
Neither of those is what you wrote, though.
If we implemented the second one of these, it would have to be controlled
by a new switch, because there are plausible use cases for wildcards that
sometimes don't match anything (not to mention backwards compatibility).
There might be a reasonable argument for the first one being the
default behavior, though; I'm not sure if we could get away with that
from a compatibility perspective.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2015-03-15 16:09 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
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 implementationin
attachment.
I think this design is seriously broken. If I have '-t foo*' the code
should not prevent that from matching multiple tables. What would the use
case for such a restriction be?
the behave is same - only one real identifier is allowed
What would make sense to me is one or both of these ideas:
* require a match for a wildcard-free -t switch
* require at least one (not "exactly one") match for a wildcarded -t
switch.Neither of those is what you wrote, though.
If we implemented the second one of these, it would have to be controlled
by a new switch, because there are plausible use cases for wildcards that
sometimes don't match anything (not to mention backwards compatibility).
There might be a reasonable argument for the first one being the
default behavior, though; I'm not sure if we could get away with that
from a compatibility perspective.
both your variant has sense for me. We can implement these points
separately. And I see a first point as much more important than second.
Because there is a significant risk of hidden broken backup.
We can implement a some long option with same functionality like now - for
somebody who need backup some explicitly specified tables optionally. Maybe
"--table-if-exists" ??
Is it acceptable for you?
Regards
Pavel
Show quoted text
regards, tom lane
Hi
2015-03-15 16:09 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
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 implementationin
attachment.
I think this design is seriously broken. If I have '-t foo*' the code
should not prevent that from matching multiple tables. What would the use
case for such a restriction be?What would make sense to me is one or both of these ideas:
* require a match for a wildcard-free -t switch
* require at least one (not "exactly one") match for a wildcarded -t
switch.
attached initial implementation
Regards
Pavel
Show quoted text
Neither of those is what you wrote, though.
If we implemented the second one of these, it would have to be controlled
by a new switch, because there are plausible use cases for wildcards that
sometimes don't match anything (not to mention backwards compatibility).
There might be a reasonable argument for the first one being the
default behavior, though; I'm not sure if we could get away with that
from a compatibility perspective.regards, tom lane
Attachments:
pg_dump-stricter.patchtext/x-patch; charset=US-ASCII; name=pg_dump-stricter.patchDownload
commit c1c7d9671a751bda1918d479b81c38c538701ec1
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Mon Mar 23 17:06:39 2015 +0100
initial
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index d7506e1..47ae6b8 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -983,7 +983,8 @@ bool
processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
- const char *altnamevar, const char *visibilityrule)
+ const char *altnamevar, const char *visibilityrule,
+ bool *with_wildcards)
{
PQExpBufferData schemabuf;
PQExpBufferData namebuf;
@@ -997,6 +998,10 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
(appendPQExpBufferStr(buf, have_where ? " AND " : "WHERE "), \
have_where = true, added_clause = true)
+#define SET_WITH_WILDCARDS(b) if (with_wildcards) *with_wildcards = b;
+
+ SET_WITH_WILDCARDS(false);
+
if (pattern == NULL)
{
/* Default: select all visible objects */
@@ -1055,11 +1060,15 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
{
appendPQExpBufferStr(&namebuf, ".*");
cp++;
+
+ SET_WITH_WILDCARDS(true);
}
else if (!inquotes && ch == '?')
{
appendPQExpBufferChar(&namebuf, '.');
cp++;
+
+ SET_WITH_WILDCARDS(true);
}
else if (!inquotes && ch == '.')
{
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index b176746..7fb7b62 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -94,7 +94,8 @@ extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf,
const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
- const char *altnamevar, const char *visibilityrule);
+ const char *altnamevar, const char *visibilityrule,
+ bool *with_wildcards);
extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
uint32 objectId, PQExpBuffer sql);
extern void emitShSecLabels(PGconn *conn, PGresult *res,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f24fefa..dd1d813 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -114,6 +114,7 @@ static SimpleStringList table_exclude_patterns = {NULL, NULL};
static SimpleOidList table_exclude_oids = {NULL, NULL};
static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
+static SimpleStringList optional_tables = {NULL, NULL};
char g_opaque_type[10]; /* name for the opaque type */
@@ -132,7 +133,7 @@ static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids);
-static void expand_table_name_patterns(Archive *fout,
+static void expand_table_name_patterns(Archive *fout, bool check_patterns,
SimpleStringList *patterns,
SimpleOidList *oids);
static NamespaceInfo *findNamespace(Archive *fout, Oid nsoid, Oid objoid);
@@ -332,6 +333,7 @@ main(int argc, char **argv)
{"section", required_argument, NULL, 5},
{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
{"snapshot", required_argument, NULL, 6},
+ {"table-if-exists", required_argument, NULL, 7},
{"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},
@@ -515,6 +517,10 @@ main(int argc, char **argv)
dumpsnapshot = pg_strdup(optarg);
break;
+ case 7: /* optional table dump */
+ simple_string_list_append(&optional_tables, optarg);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -697,17 +703,24 @@ main(int argc, char **argv)
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
- if (table_include_patterns.head != NULL)
+ if (table_include_patterns.head != NULL || optional_tables.head != NULL)
{
- expand_table_name_patterns(fout, &table_include_patterns,
+ expand_table_name_patterns(fout, true, &table_include_patterns,
+ &table_include_oids);
+
+ expand_table_name_patterns(fout, true, &table_include_patterns,
+ &table_include_oids);
+
+ expand_table_name_patterns(fout, false, &optional_tables,
&table_include_oids);
+
if (table_include_oids.head == NULL)
exit_horribly(NULL, "No matching tables were found\n");
}
- expand_table_name_patterns(fout, &table_exclude_patterns,
+ expand_table_name_patterns(fout, false, &table_exclude_patterns,
&table_exclude_oids);
- expand_table_name_patterns(fout, &tabledata_exclude_patterns,
+ expand_table_name_patterns(fout, false, &tabledata_exclude_patterns,
&tabledata_exclude_oids);
/* non-matching exclusion patterns aren't an error */
@@ -903,6 +916,7 @@ help(const char *progname)
printf(_(" --section=SECTION dump named section (pre-data, data, or post-data)\n"));
printf(_(" --serializable-deferrable wait until the dump can run without anomalies\n"));
printf(_(" --snapshot=SNAPSHOT use given synchronous snapshot for the dump\n"));
+ printf(_(" --table-if-exists=TABLE optional dump table\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
@@ -1159,7 +1173,7 @@ expand_schema_name_patterns(Archive *fout,
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
processSQLNamePattern(GetConnection(fout), query, cell->val, false,
- false, NULL, "n.nspname", NULL, NULL);
+ false, NULL, "n.nspname", NULL, NULL, NULL);
}
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -1176,20 +1190,36 @@ expand_schema_name_patterns(Archive *fout,
/*
* Find the OIDs of all tables matching the given list of patterns,
* and append them to the given OID list.
+ *
+ * When check_patterns is true, then ensure any pattern without wildcards has to specify one or
+ * more tables.
*/
static void
-expand_table_name_patterns(Archive *fout,
+expand_table_name_patterns(Archive *fout, bool check_patterns,
SimpleStringList *patterns, SimpleOidList *oids)
{
PQExpBuffer query;
+ PQExpBuffer query_nonempty_check;
+ PQExpBuffer filter;
PGresult *res;
SimpleStringListCell *cell;
int i;
+#define SEARCH_TABLE_OID_QUERY_WITH_FILTER \
+ "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" \
+ "%s", \
+ RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW, \
+ RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE
+
if (patterns->head == NULL)
return; /* nothing to do */
query = createPQExpBuffer();
+ query_nonempty_check = createPQExpBuffer();
+ filter = createPQExpBuffer();
/*
* We use UNION ALL rather than UNION; this might sometimes result in
@@ -1198,28 +1228,56 @@ expand_table_name_patterns(Archive *fout,
for (cell = patterns->head; cell; cell = cell->next)
{
- 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,
+ bool with_wildcards;
+
+ processSQLNamePattern(GetConnection(fout), filter, cell->val, true,
false, "n.nspname", "c.relname", NULL,
- "pg_catalog.pg_table_is_visible(c.oid)");
- }
+ "pg_catalog.pg_table_is_visible(c.oid)",
+ &with_wildcards);
- res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (check_patterns && !with_wildcards)
+ {
+ appendPQExpBuffer(query_nonempty_check,
+ SEARCH_TABLE_OID_QUERY_WITH_FILTER,
+ filter->data);
- for (i = 0; i < PQntuples(res); i++)
+ res = ExecuteSqlQuery(fout, query_nonempty_check->data, PGRES_TUPLES_OK);
+ if (PQntuples(res) == 0)
+ exit_horribly(NULL, "No matching table were found for \"%s\"\n", cell->val);
+
+ for (i = 0; i < PQntuples(res); i++)
+ {
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query_nonempty_check);
+ }
+ else
+ {
+ if (cell != patterns->head)
+ appendPQExpBufferStr(query, "UNION ALL\n");
+ appendPQExpBuffer(query,
+ SEARCH_TABLE_OID_QUERY_WITH_FILTER,
+ filter->data);
+ }
+
+ resetPQExpBuffer(filter);
+ }
+
+ if (*query->data != '\0')
{
- simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ 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)));
+ }
+
+ PQclear(res);
}
- PQclear(res);
+ destroyPQExpBuffer(filter);
destroyPQExpBuffer(query);
}
2015-03-23 17:11 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2015-03-15 16:09 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
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.
I think this design is seriously broken. If I have '-t foo*' the code
should not prevent that from matching multiple tables. What would the use
case for such a restriction be?What would make sense to me is one or both of these ideas:
* require a match for a wildcard-free -t switch
* require at least one (not "exactly one") match for a wildcarded -t
switch.attached initial implementation
updated version - same mechanism should be used for schema
Regards
Pavel
Show quoted text
Regards
Pavel
Neither of those is what you wrote, though.
If we implemented the second one of these, it would have to be controlled
by a new switch, because there are plausible use cases for wildcards that
sometimes don't match anything (not to mention backwards compatibility).
There might be a reasonable argument for the first one being the
default behavior, though; I'm not sure if we could get away with that
from a compatibility perspective.regards, tom lane
Attachments:
pg_dump-strict-3.patchtext/x-patch; charset=US-ASCII; name=pg_dump-strict-3.patchDownload
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
new file mode 100644
index d7506e1..47ae6b8
*** a/src/bin/pg_dump/dumputils.c
--- b/src/bin/pg_dump/dumputils.c
*************** bool
*** 983,989 ****
processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
! const char *altnamevar, const char *visibilityrule)
{
PQExpBufferData schemabuf;
PQExpBufferData namebuf;
--- 983,990 ----
processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
! const char *altnamevar, const char *visibilityrule,
! bool *with_wildcards)
{
PQExpBufferData schemabuf;
PQExpBufferData namebuf;
*************** processSQLNamePattern(PGconn *conn, PQEx
*** 997,1002 ****
--- 998,1007 ----
(appendPQExpBufferStr(buf, have_where ? " AND " : "WHERE "), \
have_where = true, added_clause = true)
+ #define SET_WITH_WILDCARDS(b) if (with_wildcards) *with_wildcards = b;
+
+ SET_WITH_WILDCARDS(false);
+
if (pattern == NULL)
{
/* Default: select all visible objects */
*************** processSQLNamePattern(PGconn *conn, PQEx
*** 1055,1065 ****
--- 1060,1074 ----
{
appendPQExpBufferStr(&namebuf, ".*");
cp++;
+
+ SET_WITH_WILDCARDS(true);
}
else if (!inquotes && ch == '?')
{
appendPQExpBufferChar(&namebuf, '.');
cp++;
+
+ SET_WITH_WILDCARDS(true);
}
else if (!inquotes && ch == '.')
{
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
new file mode 100644
index b176746..7fb7b62
*** a/src/bin/pg_dump/dumputils.h
--- b/src/bin/pg_dump/dumputils.h
*************** extern bool processSQLNamePattern(PGconn
*** 94,100 ****
const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
! const char *altnamevar, const char *visibilityrule);
extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
uint32 objectId, PQExpBuffer sql);
extern void emitShSecLabels(PGconn *conn, PGresult *res,
--- 94,101 ----
const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
! const char *altnamevar, const char *visibilityrule,
! bool *with_wildcards);
extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
uint32 objectId, PQExpBuffer sql);
extern void emitShSecLabels(PGconn *conn, PGresult *res,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index f24fefa..ff1e6a0
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** static SimpleStringList schema_include_p
*** 107,112 ****
--- 107,113 ----
static SimpleOidList schema_include_oids = {NULL, NULL};
static SimpleStringList schema_exclude_patterns = {NULL, NULL};
static SimpleOidList schema_exclude_oids = {NULL, NULL};
+ static SimpleStringList schema_optional_patterns = {NULL, NULL};
static SimpleStringList table_include_patterns = {NULL, NULL};
static SimpleOidList table_include_oids = {NULL, NULL};
*************** static SimpleStringList table_exclude_pa
*** 114,119 ****
--- 115,121 ----
static SimpleOidList table_exclude_oids = {NULL, NULL};
static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
+ static SimpleStringList table_optional_patterns = {NULL, NULL};
char g_opaque_type[10]; /* name for the opaque type */
*************** static void setup_connection(Archive *AH
*** 129,138 ****
const char *dumpencoding, const char *dumpsnapshot,
char *use_role);
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
! static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids);
! static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids);
static NamespaceInfo *findNamespace(Archive *fout, Oid nsoid, Oid objoid);
--- 131,140 ----
const char *dumpencoding, const char *dumpsnapshot,
char *use_role);
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
! static void expand_schema_name_patterns(Archive *fout, bool check_patterns,
SimpleStringList *patterns,
SimpleOidList *oids);
! static void expand_table_name_patterns(Archive *fout, bool check_patterns,
SimpleStringList *patterns,
SimpleOidList *oids);
static NamespaceInfo *findNamespace(Archive *fout, Oid nsoid, Oid objoid);
*************** main(int argc, char **argv)
*** 329,337 ****
--- 331,341 ----
{"no-tablespaces", no_argument, &dopt.outputNoTablespaces, 1},
{"quote-all-identifiers", no_argument, "e_all_identifiers, 1},
{"role", required_argument, NULL, 3},
+ {"schema-if-exists", required_argument, NULL, 7},
{"section", required_argument, NULL, 5},
{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
{"snapshot", required_argument, NULL, 6},
+ {"table-if-exists", required_argument, NULL, 8},
{"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},
*************** main(int argc, char **argv)
*** 515,520 ****
--- 519,532 ----
dumpsnapshot = pg_strdup(optarg);
break;
+ case 7: /* optional schema dump */
+ simple_string_list_append(&schema_optional_patterns, optarg);
+ break;
+
+ case 8: /* optional table dump */
+ simple_string_list_append(&table_optional_patterns, optarg);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
*************** main(int argc, char **argv)
*** 685,713 ****
}
/* Expand schema selection patterns into OID lists */
! if (schema_include_patterns.head != NULL)
{
! expand_schema_name_patterns(fout, &schema_include_patterns,
&schema_include_oids);
if (schema_include_oids.head == NULL)
exit_horribly(NULL, "No matching schemas were found\n");
}
! expand_schema_name_patterns(fout, &schema_exclude_patterns,
&schema_exclude_oids);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
! if (table_include_patterns.head != NULL)
{
! expand_table_name_patterns(fout, &table_include_patterns,
&table_include_oids);
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);
! expand_table_name_patterns(fout, &tabledata_exclude_patterns,
&tabledata_exclude_oids);
/* non-matching exclusion patterns aren't an error */
--- 697,733 ----
}
/* Expand schema selection patterns into OID lists */
! if (schema_include_patterns.head != NULL || schema_optional_patterns.head != NULL)
{
! expand_schema_name_patterns(fout, true, &schema_include_patterns,
! &schema_include_oids);
!
! expand_schema_name_patterns(fout, false, &schema_optional_patterns,
&schema_include_oids);
+
if (schema_include_oids.head == NULL)
exit_horribly(NULL, "No matching schemas were found\n");
}
! expand_schema_name_patterns(fout, false, &schema_exclude_patterns,
&schema_exclude_oids);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
! if (table_include_patterns.head != NULL || table_optional_patterns.head != NULL)
{
! expand_table_name_patterns(fout, true, &table_include_patterns,
! &table_include_oids);
!
! expand_table_name_patterns(fout, false, &table_optional_patterns,
&table_include_oids);
+
if (table_include_oids.head == NULL)
exit_horribly(NULL, "No matching tables were found\n");
}
! expand_table_name_patterns(fout, false, &table_exclude_patterns,
&table_exclude_oids);
! expand_table_name_patterns(fout, false, &tabledata_exclude_patterns,
&tabledata_exclude_oids);
/* non-matching exclusion patterns aren't an error */
*************** help(const char *progname)
*** 900,908 ****
--- 920,930 ----
printf(_(" --no-tablespaces do not dump tablespace assignments\n"));
printf(_(" --no-unlogged-table-data do not dump unlogged table data\n"));
printf(_(" --quote-all-identifiers quote all identifiers, even if not key words\n"));
+ printf(_(" --schema-if-exists=SCHEMA optional dump of schema\n"));
printf(_(" --section=SECTION dump named section (pre-data, data, or post-data)\n"));
printf(_(" --serializable-deferrable wait until the dump can run without anomalies\n"));
printf(_(" --snapshot=SNAPSHOT use given synchronous snapshot for the dump\n"));
+ printf(_(" --table-if-exists=TABLE optional dump of table\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
*************** parseArchiveFormat(const char *format, A
*** 1130,1144 ****
* and append them to the given OID list.
*/
static void
! expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids)
{
PQExpBuffer query;
PGresult *res;
SimpleStringListCell *cell;
int i;
if (patterns->head == NULL)
return; /* nothing to do */
--- 1152,1171 ----
* and append them to the given OID list.
*/
static void
! expand_schema_name_patterns(Archive *fout, bool check_patterns,
SimpleStringList *patterns,
SimpleOidList *oids)
{
PQExpBuffer query;
+ PQExpBuffer query_nonempty_check;
+ PQExpBuffer filter;
PGresult *res;
SimpleStringListCell *cell;
int i;
+ #define SEARCH_SCHEMA_OID_QUERY_WITH_FILTER \
+ "SELECT oid FROM pg_catalog.pg_namespace n\n%s"
+
if (patterns->head == NULL)
return; /* nothing to do */
*************** expand_schema_name_patterns(Archive *fou
*** 1146,1151 ****
--- 1173,1180 ----
exit_horribly(NULL, "server version must be at least 7.3 to use schema selection switches\n");
query = createPQExpBuffer();
+ query_nonempty_check = createPQExpBuffer();
+ filter = createPQExpBuffer();
/*
* We use UNION ALL rather than UNION; this might sometimes result in
*************** expand_schema_name_patterns(Archive *fou
*** 1154,1195 ****
for (cell = patterns->head; cell; cell = cell->next)
{
! if (cell != patterns->head)
! appendPQExpBufferStr(query, "UNION ALL\n");
! appendPQExpBuffer(query,
! "SELECT oid FROM pg_catalog.pg_namespace n\n");
! processSQLNamePattern(GetConnection(fout), query, cell->val, false,
! false, NULL, "n.nspname", NULL, NULL);
! }
! 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)));
}
! PQclear(res);
destroyPQExpBuffer(query);
}
/*
* 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)
{
PQExpBuffer query;
PGresult *res;
SimpleStringListCell *cell;
int i;
if (patterns->head == NULL)
return; /* nothing to do */
query = createPQExpBuffer();
/*
* We use UNION ALL rather than UNION; this might sometimes result in
--- 1183,1274 ----
for (cell = patterns->head; cell; cell = cell->next)
{
! bool with_wildcards;
! processSQLNamePattern(GetConnection(fout), filter, cell->val, false,
! false, NULL, "n.nspname", NULL, NULL,
! &with_wildcards);
! if (check_patterns && !with_wildcards)
! {
! appendPQExpBuffer(query_nonempty_check,
! SEARCH_SCHEMA_OID_QUERY_WITH_FILTER,
! filter->data);
!
! res = ExecuteSqlQuery(fout, query_nonempty_check->data, PGRES_TUPLES_OK);
! if (PQntuples(res) == 0)
! exit_horribly(NULL, "No matching schema were found for \"%s\"\n", cell->val);
!
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
!
! PQclear(res);
! resetPQExpBuffer(query_nonempty_check);
! }
! else
! {
! if (cell != patterns->head)
! appendPQExpBufferStr(query, "UNION ALL\n");
! appendPQExpBuffer(query,
! SEARCH_SCHEMA_OID_QUERY_WITH_FILTER,
! filter->data);
! }
!
! resetPQExpBuffer(filter);
! }
!
! if (*query->data != '\0')
{
! 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)));
! }
!
! PQclear(res);
}
! destroyPQExpBuffer(filter);
! destroyPQExpBuffer(query_nonempty_check);
destroyPQExpBuffer(query);
}
/*
* Find the OIDs of all tables matching the given list of patterns,
* and append them to the given OID list.
+ *
+ * When check_patterns is true, then ensure any pattern without wildcards has to specify one or
+ * more tables.
*/
static void
! expand_table_name_patterns(Archive *fout, bool check_patterns,
SimpleStringList *patterns, SimpleOidList *oids)
{
PQExpBuffer query;
+ PQExpBuffer query_nonempty_check;
+ PQExpBuffer filter;
PGresult *res;
SimpleStringListCell *cell;
int i;
+ #define SEARCH_TABLE_OID_QUERY_WITH_FILTER \
+ "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" \
+ "%s", \
+ RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW, \
+ RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE
+
if (patterns->head == NULL)
return; /* nothing to do */
query = createPQExpBuffer();
+ query_nonempty_check = createPQExpBuffer();
+ filter = createPQExpBuffer();
/*
* We use UNION ALL rather than UNION; this might sometimes result in
*************** expand_table_name_patterns(Archive *fout
*** 1198,1225 ****
for (cell = patterns->head; cell; cell = cell->next)
{
! 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)");
! }
! 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)));
}
! PQclear(res);
destroyPQExpBuffer(query);
}
--- 1277,1333 ----
for (cell = patterns->head; cell; cell = cell->next)
{
! bool with_wildcards;
!
! processSQLNamePattern(GetConnection(fout), filter, cell->val, true,
false, "n.nspname", "c.relname", NULL,
! "pg_catalog.pg_table_is_visible(c.oid)",
! &with_wildcards);
! if (check_patterns && !with_wildcards)
! {
! appendPQExpBuffer(query_nonempty_check,
! SEARCH_TABLE_OID_QUERY_WITH_FILTER,
! filter->data);
! res = ExecuteSqlQuery(fout, query_nonempty_check->data, PGRES_TUPLES_OK);
! if (PQntuples(res) == 0)
! exit_horribly(NULL, "No matching table were found for \"%s\"\n", cell->val);
!
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
!
! PQclear(res);
! resetPQExpBuffer(query_nonempty_check);
! }
! else
! {
! if (cell != patterns->head)
! appendPQExpBufferStr(query, "UNION ALL\n");
! appendPQExpBuffer(query,
! SEARCH_TABLE_OID_QUERY_WITH_FILTER,
! filter->data);
! }
!
! resetPQExpBuffer(filter);
! }
!
! if (*query->data != '\0')
{
! 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)));
! }
!
! PQclear(res);
}
! destroyPQExpBuffer(filter);
! destroyPQExpBuffer(query_nonempty_check);
destroyPQExpBuffer(query);
}
Pavel Stehule <pavel.stehule@gmail.com> writes:
2015-03-23 17:11 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2015-03-15 16:09 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
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.
I think this design is seriously broken. If I have '-t foo*' the code
should not prevent that from matching multiple tables. What would the use
case for such a restriction be?What would make sense to me is one or both of these ideas:
* require a match for a wildcard-free -t switch
* require at least one (not "exactly one") match for a wildcarded -t
switch.attached initial implementation
updated version - same mechanism should be used for schema
Hello,
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).
Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.
I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).
Please see attached patch.
--
Alex
Attachments:
pg_dump-strict-include-4.patchtext/x-diffDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
new file mode 100644
index a6e7b08..da33331
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*************** PostgreSQL documentation
*** 365,370 ****
--- 365,377 ----
<xref linkend="pg-dump-examples" endterm="pg-dump-examples-title">.
</para>
+ <para>
+ When <replaceable class="parameter">schema</replaceable> doesn't
+ specify a pattern, it must match an exact schema name in the database
+ to be dumped. In case of a pattern, the behavior is controlled
+ by <option>--strict-include</> option.
+ </para>
+
<note>
<para>
When <option>-n</> is specified, <application>pg_dump</application>
*************** PostgreSQL documentation
*** 515,520 ****
--- 522,534 ----
</para>
<para>
+ When <replaceable class="parameter">table</replaceable> doesn't
+ specify a pattern, it must match an exact table name in the database
+ to be dumped. In case of a pattern, the behavior is controlled
+ by <option>--strict-include</> option.
+ </para>
+
+ <para>
The <option>-n</> and <option>-N</> switches have no effect when
<option>-t</> is used, because tables selected by <option>-t</> will
be dumped regardless of those switches, and non-table objects will not
*************** PostgreSQL documentation
*** 902,907 ****
--- 916,938 ----
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>--strict-include</></term>
+ <listitem>
+ <para>
+ Require that table and/or schema include patterns match at least one
+ entity each. If any of the include patterns specified doesn't match
+ any entity in the database to be dumped, an error message is printed
+ and dump is aborted.
+ </para>
+ <para>
+ This option has no effect on the exclude table and schema patterns
+ (and also <option>--exclude-table-data</>): not matching any entities
+ isn't considered an error.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><option>--use-set-session-authorization</></term>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
new file mode 100644
index 80df8fc..f384c20
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
*************** typedef struct _dumpOptions
*** 178,183 ****
--- 178,184 ----
int outputNoTablespaces;
int use_setsessauth;
int enable_row_security;
+ int strict_include;
/* default, if no "inclusion" switches appear, is to dump everything */
bool include_everything;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index dccb472..4f62085
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** static void setup_connection(Archive *AH
*** 131,140 ****
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids);
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);
--- 131,142 ----
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict);
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);
*************** main(int argc, char **argv)
*** 333,338 ****
--- 335,341 ----
{"section", required_argument, NULL, 5},
{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
{"snapshot", required_argument, NULL, 6},
+ {"strict-include", no_argument, &dopt.strict_include, 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},
*************** main(int argc, char **argv)
*** 689,715 ****
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
! &schema_include_oids);
! if (schema_include_oids.head == NULL)
! exit_horribly(NULL, "No matching schemas were found\n");
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
! &schema_exclude_oids);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
if (table_include_patterns.head != NULL)
{
expand_table_name_patterns(fout, &table_include_patterns,
! &table_include_oids);
! 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);
expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! &tabledata_exclude_oids);
/* non-matching exclusion patterns aren't an error */
--- 692,714 ----
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
! &schema_include_oids, dopt.strict_include);
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
! &schema_exclude_oids, true);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
if (table_include_patterns.head != NULL)
{
expand_table_name_patterns(fout, &table_include_patterns,
! &table_include_oids, dopt.strict_include);
}
expand_table_name_patterns(fout, &table_exclude_patterns,
! &table_exclude_oids, true);
expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! &tabledata_exclude_oids, true);
/* non-matching exclusion patterns aren't an error */
*************** help(const char *progname)
*** 904,909 ****
--- 903,910 ----
printf(_(" --section=SECTION dump named section (pre-data, data, or post-data)\n"));
printf(_(" --serializable-deferrable wait until the dump can run without anomalies\n"));
printf(_(" --snapshot=SNAPSHOT use given synchronous snapshot for the dump\n"));
+ printf(_(" --strict-include require table and/or schema include patterns to\n"
+ " match at least one entity each\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
*************** parseArchiveFormat(const char *format, A
*** 1133,1139 ****
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids)
{
PQExpBuffer query;
PGresult *res;
--- 1134,1141 ----
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict)
{
PQExpBuffer query;
PGresult *res;
*************** expand_schema_name_patterns(Archive *fou
*** 1155,1176 ****
for (cell = patterns->head; cell; cell = cell->next)
{
! if (cell != patterns->head)
! appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
processSQLNamePattern(GetConnection(fout), query, cell->val, false,
false, NULL, "n.nspname", NULL, NULL);
! }
! 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)));
! }
! PQclear(res);
destroyPQExpBuffer(query);
}
--- 1157,1183 ----
for (cell = patterns->head; cell; cell = cell->next)
{
! resetPQExpBuffer(query);
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
+
processSQLNamePattern(GetConnection(fout), query, cell->val, false,
false, NULL, "n.nspname", NULL, NULL);
! res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
! if (PQntuples(res) < 1 &&
! (strict || strcspn(cell->val, "?*") == strlen(cell->val)))
! {
! exit_horribly(NULL, "Schema(s) not found: %s\n", cell->val);
! }
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
! PQclear(res);
! }
destroyPQExpBuffer(query);
}
*************** expand_schema_name_patterns(Archive *fou
*** 1180,1186 ****
*/
static void
expand_table_name_patterns(Archive *fout,
! SimpleStringList *patterns, SimpleOidList *oids)
{
PQExpBuffer query;
PGresult *res;
--- 1187,1194 ----
*/
static void
expand_table_name_patterns(Archive *fout,
! SimpleStringList *patterns, SimpleOidList *oids,
! bool strict)
{
PQExpBuffer query;
PGresult *res;
*************** expand_table_name_patterns(Archive *fout
*** 1199,1206 ****
for (cell = patterns->head; cell; cell = cell->next)
{
! if (cell != patterns->head)
! appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
--- 1207,1213 ----
for (cell = patterns->head; cell; cell = cell->next)
{
! resetPQExpBuffer(query);
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
*************** expand_table_name_patterns(Archive *fout
*** 1208,1226 ****
"\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)");
! }
! 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)));
}
- PQclear(res);
destroyPQExpBuffer(query);
}
--- 1215,1240 ----
"\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)");
! res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
! if (PQntuples(res) < 1 &&
! (strict || strcspn(cell->val, "?*") == strlen(cell->val)))
! {
! exit_horribly(NULL, "Table(s) not found: %s\n", cell->val);
! }
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
!
! PQclear(res);
}
destroyPQExpBuffer(query);
}
2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <oleksandr.shulgin@zalando.de>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
2015-03-23 17:11 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
2015-03-15 16:09 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
other variant, I hope better than previous. We can introduce new long
option "--strict". With this active option, every pattern specifiedby
-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.
I think this design is seriously broken. If I have '-t foo*' the code
should not prevent that from matching multiple tables. What would theuse
case for such a restriction be?
What would make sense to me is one or both of these ideas:
* require a match for a wildcard-free -t switch
* require at least one (not "exactly one") match for a wildcarded -t
switch.attached initial implementation
updated version - same mechanism should be used for schema
Hello,
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).
it was prototype - I believe so issue with describe.c can be solved better
Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).
hard to say - any variant has own advantages and disadvantages
But I more to unlike it than like - it is more usual, when you use exact
name so, you need it exactly one, and when you use some wildcard, so you
are expecting one or more tables.
This use case is not checked in your patch.
Regards
Pavel
Show quoted text
Please see attached patch.
--
Alex
On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <oleksandr.shulgin@zalando.de
:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).it was prototype - I believe so issue with describe.c can be solved better
Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).hard to say - any variant has own advantages and disadvantages
But I more to unlike it than like - it is more usual, when you use exact
name so, you need it exactly one, and when you use some wildcard, so you
are expecting one or more tables.This use case is not checked in your patch.
Maybe I'm missing something, but I believe it's handled by
pg_dump -t mytables* --strict-include
so that it will error out if nothing was found for mytables* pattern.
--
Alex
2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr <oleksandr.shulgin@zalando.de>
:
On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
oleksandr.shulgin@zalando.de>:I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).it was prototype - I believe so issue with describe.c can be solved better
Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).hard to say - any variant has own advantages and disadvantages
But I more to unlike it than like - it is more usual, when you use exact
name so, you need it exactly one, and when you use some wildcard, so you
are expecting one or more tables.This use case is not checked in your patch.
Maybe I'm missing something, but I believe it's handled by
pg_dump -t mytables* --strict-include
so that it will error out if nothing was found for mytables* pattern.
If I understand it raise a error when it found more than one table
Pavel
Show quoted text
--
Alex
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).
Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.
I'm pretty sure we had agreed *not* to change the default behavior of -t.
I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).
If we do it as a separate option, then it necessarily changes the behavior
for *each* -t switch in the call. Can anyone show a common use-case where
that's no good, and you need separate behavior for each of several -t
switches? If not, I like the simplicity of this approach. (Perhaps the
switch name could use some bikeshedding, though.)
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, May 22, 2015 at 6:32 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr <
oleksandr.shulgin@zalando.de>:On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
oleksandr.shulgin@zalando.de>:I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).it was prototype - I believe so issue with describe.c can be solved
betterAlso, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).hard to say - any variant has own advantages and disadvantages
But I more to unlike it than like - it is more usual, when you use exact
name so, you need it exactly one, and when you use some wildcard, so you
are expecting one or more tables.This use case is not checked in your patch.
Maybe I'm missing something, but I believe it's handled by
pg_dump -t mytables* --strict-include
so that it will error out if nothing was found for mytables* pattern.
If I understand it raise a error when it found more than one table
I hope not, and that totally was not my intent :-p
It should raise if it found *less than* one, that is: none.
2015-05-22 18:35 GMT+02:00 Shulgin, Oleksandr <oleksandr.shulgin@zalando.de>
:
On Fri, May 22, 2015 at 6:32 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr <
oleksandr.shulgin@zalando.de>:On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
oleksandr.shulgin@zalando.de>:I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).it was prototype - I believe so issue with describe.c can be solved
betterAlso, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I propose instead to add a separate new option --strict-include,
without
argument, that only controls the behavior when an include pattern
didn't
find any table (or schema).hard to say - any variant has own advantages and disadvantages
But I more to unlike it than like - it is more usual, when you use
exact name so, you need it exactly one, and when you use some wildcard, so
you are expecting one or more tables.This use case is not checked in your patch.
Maybe I'm missing something, but I believe it's handled by
pg_dump -t mytables* --strict-include
so that it will error out if nothing was found for mytables* pattern.
If I understand it raise a error when it found more than one table
I hope not, and that totally was not my intent :-p
It should raise if it found *less than* one, that is: none.
ok, then I have not objection
2015-05-22 18:34 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I'm pretty sure we had agreed *not* to change the default behavior of -t.
I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).If we do it as a separate option, then it necessarily changes the behavior
for *each* -t switch in the call. Can anyone show a common use-case where
that's no good, and you need separate behavior for each of several -t
switches? If not, I like the simplicity of this approach. (Perhaps the
switch name could use some bikeshedding, though.)
it is near to one proposal
implement only new long option "--required-table"
Pavel
Show quoted text
regards, tom lane
On Fri, May 22, 2015 at 6:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I'm pretty sure we had agreed *not* to change the default behavior of -t.
My patch does that, in the case of no-wildcards -t argument.
However, it can be fixed easily: just drop that strcspn() call, and then
default behavior is the same for both wildcard and exact matches, since
--strict-include is off by default.
--
Alex
On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2015-05-22 18:34 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I'm pretty sure we had agreed *not* to change the default behavior of -t.
I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).If we do it as a separate option, then it necessarily changes the behavior
for *each* -t switch in the call. Can anyone show a common use-case where
that's no good, and you need separate behavior for each of several -t
switches? If not, I like the simplicity of this approach. (Perhaps the
switch name could use some bikeshedding, though.)it is near to one proposal
implement only new long option "--required-table"
There is no updated version of the patch. So I marked this patch
as "Waiting on Author".
One very simple question is, doesn't -n option have very similar problem?
Also what about -N, -T and --exclude-table-data? Not sure if we need to
handle them in the similar way as you proposed.
Isn't it sufficient to only emit the warning message to stderr if there
is no table matching the pattern specified in -t?
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2015-07-08 5:36 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2015-05-22 18:34 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.I'm pretty sure we had agreed *not* to change the default behavior of
-t.
I propose instead to add a separate new option --strict-include,
without
argument, that only controls the behavior when an include pattern
didn't
find any table (or schema).
If we do it as a separate option, then it necessarily changes the
behavior
for *each* -t switch in the call. Can anyone show a common use-case
where
that's no good, and you need separate behavior for each of several -t
switches? If not, I like the simplicity of this approach. (Perhaps the
switch name could use some bikeshedding, though.)it is near to one proposal
implement only new long option "--required-table"
There is no updated version of the patch. So I marked this patch
as "Waiting on Author".
tomorrow I'll return to this topic.
One very simple question is, doesn't -n option have very similar problem?
Also what about -N, -T and --exclude-table-data? Not sure if we need to
handle them in the similar way as you proposed.
hard to say - I understand to your motivation, and it can signalize some
inconsistency in environment, but it has not same negative impact as same
problem with -t -n options.
This is maybe place for warning message with possibility to disable this
warning. But maybe it is premature optimization?
Next way is introduction of "strictness" option - default can be equivalent
of current, safe can check only tables required for dump, strict can check
all.
Isn't it sufficient to only emit the warning message to stderr if there
is no table matching the pattern specified in -t?
I prefer raising error in this case.
1. I am thinking so missing tables for dump signalize important
inconsistency in environment and it is important bug
2. The warning is not simply detected (and processed) in bash scripts.
Regards
Pavel
Show quoted text
Regards,
--
Fujii Masao
Hi
I am sending updated version. It implements new long option
"--strict-names". If this option is used, then for any entered name
(without any wildcard char) must be found least one object. This option has
not impact on patters (has wildcards chars). When this option is not used,
then behave is 100% same as before (with same numbers of SQL queries for -t
option). It is based on Oleksandr's documentation (and lightly modified
philosophy), and cleaned my previous patch. A test on wildchard existency
"strcspn(cell->val, "?*")" cannot be used, because it doesn't calculate
quotes (but a replace has few lines only).
There is a possibility to remove a wildcard char test and require least one
entry for patters too. But I am thinking, when somebody explicitly uses any
wildcard, then he calculate with a possibility of empty result.
Regards
Pavel
2015-07-09 22:48 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
Show quoted text
2015-07-08 5:36 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2015-05-22 18:34 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).Also, the new --table-if-exists options seems to be doing what the
old
--table did, and I'm not really sure I underestand what --table does
now.I'm pretty sure we had agreed *not* to change the default behavior of
-t.
I propose instead to add a separate new option --strict-include,
without
argument, that only controls the behavior when an include pattern
didn't
find any table (or schema).
If we do it as a separate option, then it necessarily changes the
behavior
for *each* -t switch in the call. Can anyone show a common use-case
where
that's no good, and you need separate behavior for each of several -t
switches? If not, I like the simplicity of this approach. (Perhapsthe
switch name could use some bikeshedding, though.)
it is near to one proposal
implement only new long option "--required-table"
There is no updated version of the patch. So I marked this patch
as "Waiting on Author".tomorrow I'll return to this topic.
One very simple question is, doesn't -n option have very similar problem?
Also what about -N, -T and --exclude-table-data? Not sure if we need to
handle them in the similar way as you proposed.hard to say - I understand to your motivation, and it can signalize some
inconsistency in environment, but it has not same negative impact as same
problem with -t -n options.This is maybe place for warning message with possibility to disable this
warning. But maybe it is premature optimization?Next way is introduction of "strictness" option - default can be
equivalent of current, safe can check only tables required for dump, strict
can check all.Isn't it sufficient to only emit the warning message to stderr if there
is no table matching the pattern specified in -t?I prefer raising error in this case.
1. I am thinking so missing tables for dump signalize important
inconsistency in environment and it is important bug
2. The warning is not simply detected (and processed) in bash scripts.Regards
Pavel
Regards,
--
Fujii Masao
Attachments:
pg_dump-strict-5.patchtext/x-patch; charset=US-ASCII; name=pg_dump-strict-5.patchDownload
commit 2f54f7ea786744540d9176cecc086cfbf32e7695
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Sun Jul 19 20:30:32 2015 +0200
initial
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 7467e86..423757d 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -493,6 +493,22 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--strict-names</></term>
+ <listitem>
+ <para>
+ Require that table and/or schema without wildcard chars match
+ at least one entity each. Without any entity in the database
+ to be dumped, an error message is printed and dump is aborted.
+ </para>
+ <para>
+ This option has no effect on the exclude table and schema patterns
+ (and also <option>--exclude-table-data</>): not matching any entities
+ isn't considered an error.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-t <replaceable class="parameter">table</replaceable></option></term>
<term><option>--table=<replaceable class="parameter">table</replaceable></option></term>
<listitem>
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index d7506e1..598df0b 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -955,9 +955,9 @@ AddAcl(PQExpBuffer aclbuf, const char *keyword, const char *subname)
/*
- * processSQLNamePattern
+ * processSQLName
*
- * Scan a wildcard-pattern string and generate appropriate WHERE clauses
+ * Scan a possible wildcard-pattern string and generate appropriate WHERE clauses
* to limit the set of objects returned. The WHERE clauses are appended
* to the already-partially-constructed query in buf. Returns whether
* any clause was added.
@@ -978,12 +978,16 @@ AddAcl(PQExpBuffer aclbuf, const char *keyword, const char *subname)
*
* Formatting note: the text already present in buf should end with a newline.
* The appended text, if any, will end with one too.
+ *
+ * found_wildcard_char: pointer to boolean pointer, it is true, when name
+ * contains some wildcard char.
*/
bool
-processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
+processSQLName(PGconn *conn, PQExpBuffer buf, const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
- const char *altnamevar, const char *visibilityrule)
+ const char *altnamevar, const char *visibilityrule,
+ bool *found_wildcard_char)
{
PQExpBufferData schemabuf;
PQExpBufferData namebuf;
@@ -993,6 +997,8 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
int i;
bool added_clause = false;
+ *found_wildcard_char = false;
+
#define WHEREAND() \
(appendPQExpBufferStr(buf, have_where ? " AND " : "WHERE "), \
have_where = true, added_clause = true)
@@ -1054,11 +1060,13 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
else if (!inquotes && ch == '*')
{
appendPQExpBufferStr(&namebuf, ".*");
+ *found_wildcard_char = true;
cp++;
}
else if (!inquotes && ch == '?')
{
appendPQExpBufferChar(&namebuf, '.');
+ *found_wildcard_char = true;
cp++;
}
else if (!inquotes && ch == '.')
@@ -1167,6 +1175,25 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
}
/*
+ * processSQLNamePattern
+ *
+ * Same as processSQLName but there are no important fact, if some wildcard is used or not.
+ */
+bool
+processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
+ bool have_where, bool force_escape,
+ const char *schemavar, const char *namevar,
+ const char *altnamevar, const char *visibilityrule)
+{
+ bool found_wildcard_char;
+
+ return processSQLName(conn, buf, pattern,
+ have_where, force_escape,
+ schemavar, namevar,
+ altnamevar, visibilityrule, &found_wildcard_char);
+}
+
+/*
* buildShSecLabelQuery
*
* Build a query to retrieve security labels for a shared object.
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index b176746..cef2a8b 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -90,6 +90,11 @@ extern bool buildDefaultACLCommands(const char *type, const char *nspname,
const char *acls, const char *owner,
int remoteVersion,
PQExpBuffer sql);
+extern bool processSQLName(PGconn *conn, PQExpBuffer buf, const char *pattern,
+ bool have_where, bool force_escape,
+ const char *schemavar, const char *namevar,
+ const char *altnamevar, const char *visibilityrule,
+ bool *found_wildcard_char);
extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf,
const char *pattern,
bool have_where, bool force_escape,
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 80df8fc..4109039 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -178,6 +178,7 @@ typedef struct _dumpOptions
int outputNoTablespaces;
int use_setsessauth;
int enable_row_security;
+ int strict_names;
/* default, if no "inclusion" switches appear, is to dump everything */
bool include_everything;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 0e036b8..24931e8 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -131,10 +131,12 @@ static void setup_connection(Archive *AH, DumpOptions *dopt,
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids);
+ SimpleOidList *oids,
+ bool strict_names);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids);
+ SimpleOidList *oids,
+ bool strict_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);
@@ -332,6 +334,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-names", no_argument, &dopt.strict_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},
@@ -684,27 +687,32 @@ main(int argc, char **argv)
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
- &schema_include_oids);
+ &schema_include_oids,
+ dopt.strict_names);
if (schema_include_oids.head == NULL)
exit_horribly(NULL, "No matching schemas were found\n");
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
- &schema_exclude_oids);
+ &schema_exclude_oids,
+ false);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
if (table_include_patterns.head != NULL)
{
expand_table_name_patterns(fout, &table_include_patterns,
- &table_include_oids);
+ &table_include_oids,
+ dopt.strict_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 */
@@ -899,6 +907,8 @@ help(const char *progname)
printf(_(" --section=SECTION dump named section (pre-data, data, or post-data)\n"));
printf(_(" --serializable-deferrable wait until the dump can run without anomalies\n"));
printf(_(" --snapshot=SNAPSHOT use given synchronous snapshot for the dump\n"));
+ printf(_(" --strict-names require table and/or schema include patterns to\n"
+ " match at least one entity each\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
@@ -1129,7 +1139,8 @@ parseArchiveFormat(const char *format, ArchiveMode *mode)
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids)
+ SimpleOidList *oids,
+ bool strict_names)
{
PQExpBuffer query;
PGresult *res;
@@ -1145,28 +1156,37 @@ expand_schema_name_patterns(Archive *fout,
query = createPQExpBuffer();
/*
- * We use UNION ALL rather than UNION; this might sometimes result in
- * duplicate entries in the OID list, but we don't care.
+ * We use more queries with possible duplicate entries in the OID list,
+ * but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
- if (cell != patterns->head)
- appendPQExpBufferStr(query, "UNION ALL\n");
+ bool found_wildcard_char;
+
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
- processSQLNamePattern(GetConnection(fout), query, cell->val, false,
- false, NULL, "n.nspname", NULL, NULL);
- }
- res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ processSQLName(GetConnection(fout), query, cell->val, false,
+ false, NULL, "n.nspname", NULL, NULL,
+ &found_wildcard_char);
- for (i = 0; i < PQntuples(res); i++)
- {
- simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (PQntuples(res) > 0)
+ {
+ for (i = 0; i < PQntuples(res); i++)
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+ else
+ {
+ if (strict_names && !found_wildcard_char)
+ exit_horribly(NULL, "Schema \"%s\" not found.\n", cell->val);
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
}
- PQclear(res);
destroyPQExpBuffer(query);
}
@@ -1176,8 +1196,10 @@ expand_schema_name_patterns(Archive *fout,
*/
static void
expand_table_name_patterns(Archive *fout,
- SimpleStringList *patterns, SimpleOidList *oids)
+ SimpleStringList *patterns, SimpleOidList *oids,
+ bool strict_names)
{
+ PQExpBuffer unioned_query;
PQExpBuffer query;
PGresult *res;
SimpleStringListCell *cell;
@@ -1186,6 +1208,7 @@ expand_table_name_patterns(Archive *fout,
if (patterns->head == NULL)
return; /* nothing to do */
+ unioned_query = createPQExpBuffer();
query = createPQExpBuffer();
/*
@@ -1195,8 +1218,8 @@ expand_table_name_patterns(Archive *fout,
for (cell = patterns->head; cell; cell = cell->next)
{
- if (cell != patterns->head)
- appendPQExpBufferStr(query, "UNION ALL\n");
+ bool found_wildcard_char;
+
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
@@ -1204,19 +1227,47 @@ expand_table_name_patterns(Archive *fout,
"\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,
+ processSQLName(GetConnection(fout), query, cell->val, true,
false, "n.nspname", "c.relname", NULL,
- "pg_catalog.pg_table_is_visible(c.oid)");
- }
+ "pg_catalog.pg_table_is_visible(c.oid)",
+ &found_wildcard_char);
- res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (strict_names && !found_wildcard_char)
+ {
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+
+ if (PQntuples(res) > 0)
+ {
+ for (i = 0; i < PQntuples(res); i++)
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+ else
+ exit_horribly(NULL, "Table \"%s\" not found.\n", cell->val);
+
+ PQclear(res);
+ }
+ else
+ {
+ if (unioned_query->len > 0)
+ appendPQExpBufferStr(unioned_query, "UNION ALL\n");
+
+ appendPQExpBuffer(unioned_query, query->data, query->len);
+ }
+
+ resetPQExpBuffer(query);
+ }
- for (i = 0; i < PQntuples(res); i++)
+ if (unioned_query->len > 0)
{
- simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ res = ExecuteSqlQuery(fout, unioned_query->data, PGRES_TUPLES_OK);
+
+ for (i = 0; i < PQntuples(res); i++)
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+
+ PQclear(res);
}
- PQclear(res);
+ destroyPQExpBuffer(unioned_query);
destroyPQExpBuffer(query);
}
2015-07-19 20:54 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
I am sending updated version. It implements new long option
"--strict-names". If this option is used, then for any entered name
(without any wildcard char) must be found least one object. This option has
not impact on patters (has wildcards chars). When this option is not used,
then behave is 100% same as before (with same numbers of SQL queries for -t
option). It is based on Oleksandr's documentation (and lightly modified
philosophy), and cleaned my previous patch. A test on wildchard existency
"strcspn(cell->val, "?*")" cannot be used, because it doesn't calculate
quotes (but a replace has few lines only).There is a possibility to remove a wildcard char test and require least
one entry for patters too. But I am thinking, when somebody explicitly uses
any wildcard, then he calculate with a possibility of empty result.
other variant is using --strict-names behave as default (and implement
negative option like --disable-strict-names or some similar).
Show quoted text
Regards
Pavel
2015-07-09 22:48 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
2015-07-08 5:36 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2015-05-22 18:34 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).Also, the new --table-if-exists options seems to be doing what the
old
--table did, and I'm not really sure I underestand what --table does
now.I'm pretty sure we had agreed *not* to change the default behavior of
-t.
I propose instead to add a separate new option --strict-include,
without
argument, that only controls the behavior when an include pattern
didn't
find any table (or schema).
If we do it as a separate option, then it necessarily changes the
behavior
for *each* -t switch in the call. Can anyone show a common use-case
where
that's no good, and you need separate behavior for each of several -t
switches? If not, I like the simplicity of this approach. (Perhapsthe
switch name could use some bikeshedding, though.)
it is near to one proposal
implement only new long option "--required-table"
There is no updated version of the patch. So I marked this patch
as "Waiting on Author".tomorrow I'll return to this topic.
One very simple question is, doesn't -n option have very similar problem?
Also what about -N, -T and --exclude-table-data? Not sure if we need to
handle them in the similar way as you proposed.hard to say - I understand to your motivation, and it can signalize some
inconsistency in environment, but it has not same negative impact as same
problem with -t -n options.This is maybe place for warning message with possibility to disable this
warning. But maybe it is premature optimization?Next way is introduction of "strictness" option - default can be
equivalent of current, safe can check only tables required for dump, strict
can check all.Isn't it sufficient to only emit the warning message to stderr if there
is no table matching the pattern specified in -t?I prefer raising error in this case.
1. I am thinking so missing tables for dump signalize important
inconsistency in environment and it is important bug
2. The warning is not simply detected (and processed) in bash scripts.Regards
Pavel
Regards,
--
Fujii Masao
2015-07-19 21:08 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
2015-07-19 20:54 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
I am sending updated version. It implements new long option
"--strict-names". If this option is used, then for any entered name
(without any wildcard char) must be found least one object. This option has
not impact on patters (has wildcards chars). When this option is not used,
then behave is 100% same as before (with same numbers of SQL queries for -t
option). It is based on Oleksandr's documentation (and lightly modified
philosophy), and cleaned my previous patch. A test on wildchard existency
"strcspn(cell->val, "?*")" cannot be used, because it doesn't calculate
quotes (but a replace has few lines only).There is a possibility to remove a wildcard char test and require least
one entry for patters too. But I am thinking, when somebody explicitly uses
any wildcard, then he calculate with a possibility of empty result.other variant is using --strict-names behave as default (and implement
negative option like --disable-strict-names or some similar).
Note: originally I though, we have to fix it and change the default behave.
But with special option, we don't need it. This option in help is signal
for user, so some is risky.
Pavel
Show quoted text
Regards
Pavel
2015-07-09 22:48 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
2015-07-08 5:36 GMT+02:00 Fujii Masao <masao.fujii@gmail.com>:
On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2015-05-22 18:34 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
Oleksandr Shulgin <oleksandr.shulgin@zalando.de> writes:
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).Also, the new --table-if-exists options seems to be doing what the
old
--table did, and I'm not really sure I underestand what --table
does
now.
I'm pretty sure we had agreed *not* to change the default behavior
of -t.
I propose instead to add a separate new option --strict-include,
without
argument, that only controls the behavior when an include pattern
didn't
find any table (or schema).
If we do it as a separate option, then it necessarily changes the
behavior
for *each* -t switch in the call. Can anyone show a common use-case
where
that's no good, and you need separate behavior for each of several -t
switches? If not, I like the simplicity of this approach. (Perhapsthe
switch name could use some bikeshedding, though.)
it is near to one proposal
implement only new long option "--required-table"
There is no updated version of the patch. So I marked this patch
as "Waiting on Author".tomorrow I'll return to this topic.
One very simple question is, doesn't -n option have very similar
problem?
Also what about -N, -T and --exclude-table-data? Not sure if we need to
handle them in the similar way as you proposed.hard to say - I understand to your motivation, and it can signalize some
inconsistency in environment, but it has not same negative impact as same
problem with -t -n options.This is maybe place for warning message with possibility to disable this
warning. But maybe it is premature optimization?Next way is introduction of "strictness" option - default can be
equivalent of current, safe can check only tables required for dump, strict
can check all.Isn't it sufficient to only emit the warning message to stderr if there
is no table matching the pattern specified in -t?I prefer raising error in this case.
1. I am thinking so missing tables for dump signalize important
inconsistency in environment and it is important bug
2. The warning is not simply detected (and processed) in bash scripts.Regards
Pavel
Regards,
--
Fujii Masao
Hello,
2015-07-19 20:54 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
I am sending updated version. It implements new long option
"--strict-names". If this option is used, then for any entered name
(without any wildcard char) must be found least one object. This option has
not impact on patters (has wildcards chars).
I share the same option with Tom that it should behave in same
way regardless of the appearance of wildcards.
You replied Tom as this
the behave is same - only one real identifier is allowed
But the description above about this patch apparently says that
they are differently handled. And
expand_(schema|table)_name_patterns does the further differently
thing from both of Tom's suggestion and what mentioned in your
reply to that. I will mention for this topic again in this mail.
# Might "only one real ident is allowed" mean "at most one
# match", but not "exactly one match"?
They do like this when strict-names.
- Not allow no match for non-wildcarded names.
- Allow no match for any wildcarded name spec and finally
allowing *all* of them don't match anyting.
This looks to me quite confusing.
When this option is not used,
then behave is 100% same as before (with same numbers of SQL queries for -t
option). It is based on Oleksandr's documentation (and lightly modified
philosophy), and cleaned my previous patch. A test on wildchard existency
"strcspn(cell->val, "?*")" cannot be used, because it doesn't calculate
quotes (but a replace has few lines only).
The new name "processSQLName" looks a bit
bogus. processSQLNameIntenral would be a name commonly seen in
such cases.
There is a possibility to remove a wildcard char test and require least
one entry for patters too. But I am thinking, when somebody explicitly uses
any wildcard, then he calculate with a possibility of empty result.
Why do you think so? Wild cards are usually used to glob multiple
names at once. One or more matches are expected for many or
perhaps most cases, I think. Since so, if someone anticipate that
some of his patterns have no match, I think he shouldn't specify
--strict-names option at all.
Furthermore, I don't think no one wants to use both wildcarded
and non-wildcarded name specs at once but this is a little out of
scope.
I'd like to have opinions from others about this point.
other variant is using --strict-names behave as default (and implement
negative option like --disable-strict-names or some similar).
This contradicts Josh's request. (which I'm not totally agree:p)
Note: originally I though, we have to fix it and change the default behave.
But with special option, we don't need it. This option in help is signal
for user, so some is risky.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Sorry for the bogus on bogus.
At Thu, 23 Jul 2015 14:22:59 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20150723.142259.200902861.horiguchi.kyotaro@lab.ntt.co.jp>
Hello,
2015-07-19 20:54 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
I am sending updated version. It implements new long option
"--strict-names". If this option is used, then for any entered name
(without any wildcard char) must be found least one object. This option has
not impact on patters (has wildcards chars).I share the same option with Tom that it should behave in same
way regardless of the appearance of wildcards.You replied Tom as this
the behave is same - only one real identifier is allowed
But the description above about this patch apparently says that
they are differently handled. And
expand_(schema|table)_name_patterns does the further differently
thing from both of Tom's suggestion and what mentioned in your
reply to that. I will mention for this topic again in this mail.# Might "only one real ident is allowed" mean "at most one
# match", but not "exactly one match"?They do like this when strict-names.
- Not allow no match for non-wildcarded names.
- Allow no match for any wildcarded name spec and finally
allowing *all* of them don't match anyting.This looks to me quite confusing.
When this option is not used,
then behave is 100% same as before (with same numbers of SQL queries for -t
option). It is based on Oleksandr's documentation (and lightly modified
philosophy), and cleaned my previous patch. A test on wildchard existency
"strcspn(cell->val, "?*")" cannot be used, because it doesn't calculate
quotes (but a replace has few lines only).The new name "processSQLName" looks a bit
bogus. processSQLNameIntenral would be a name commonly seen in
such cases.
The ocrrent name is not that but processSQLNamePatternInternal.
There is a possibility to remove a wildcard char test and require least
one entry for patters too. But I am thinking, when somebody explicitly uses
any wildcard, then he calculate with a possibility of empty result.Why do you think so? Wild cards are usually used to glob multiple
names at once. One or more matches are expected for many or
perhaps most cases, I think. Since so, if someone anticipate that
some of his patterns have no match, I think he shouldn't specify
--strict-names option at all.Furthermore, I don't think no one wants to use both wildcarded
and non-wildcarded name specs at once but this is a little out of
scope.I'd like to have opinions from others about this point.
other variant is using --strict-names behave as default (and implement
negative option like --disable-strict-names or some similar).This contradicts Josh's request. (which I'm not totally agree:p)
Note: originally I though, we have to fix it and change the default behave.
But with special option, we don't need it. This option in help is signal
for user, so some is risky.regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
I am sending a new patch - without checking wildcard chars.
Regards
Pavel
2015-07-23 7:22 GMT+02:00 Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp
Show quoted text
:
Hello,
2015-07-19 20:54 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
I am sending updated version. It implements new long option
"--strict-names". If this option is used, then for any entered name
(without any wildcard char) must be found least one object. Thisoption has
not impact on patters (has wildcards chars).
I share the same option with Tom that it should behave in same
way regardless of the appearance of wildcards.You replied Tom as this
the behave is same - only one real identifier is allowed
But the description above about this patch apparently says that
they are differently handled. And
expand_(schema|table)_name_patterns does the further differently
thing from both of Tom's suggestion and what mentioned in your
reply to that. I will mention for this topic again in this mail.# Might "only one real ident is allowed" mean "at most one
# match", but not "exactly one match"?They do like this when strict-names.
- Not allow no match for non-wildcarded names.
- Allow no match for any wildcarded name spec and finally
allowing *all* of them don't match anyting.This looks to me quite confusing.
When this option is not used,
then behave is 100% same as before (with same numbers of SQL queriesfor -t
option). It is based on Oleksandr's documentation (and lightly
modified
philosophy), and cleaned my previous patch. A test on wildchard
existency
"strcspn(cell->val, "?*")" cannot be used, because it doesn't
calculate
quotes (but a replace has few lines only).
The new name "processSQLName" looks a bit
bogus. processSQLNameIntenral would be a name commonly seen in
such cases.There is a possibility to remove a wildcard char test and require
least
one entry for patters too. But I am thinking, when somebody
explicitly uses
any wildcard, then he calculate with a possibility of empty result.
Why do you think so? Wild cards are usually used to glob multiple
names at once. One or more matches are expected for many or
perhaps most cases, I think. Since so, if someone anticipate that
some of his patterns have no match, I think he shouldn't specify
--strict-names option at all.Furthermore, I don't think no one wants to use both wildcarded
and non-wildcarded name specs at once but this is a little out of
scope.I'd like to have opinions from others about this point.
other variant is using --strict-names behave as default (and implement
negative option like --disable-strict-names or some similar).This contradicts Josh's request. (which I'm not totally agree:p)
Note: originally I though, we have to fix it and change the default
behave.
But with special option, we don't need it. This option in help is signal
for user, so some is risky.regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
pg_dump-strict-6.patchtext/x-patch; charset=US-ASCII; name=pg_dump-strict-6.patchDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
new file mode 100644
index 7467e86..7c071fb
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*************** PostgreSQL documentation
*** 545,550 ****
--- 545,566 ----
</varlistentry>
<varlistentry>
+ <term><option>--strict-names</></term>
+ <listitem>
+ <para>
+ Require that table and/or schema match at least one entity each.
+ Without any entity in the database to be dumped, an error message
+ is printed and dump is aborted.
+ </para>
+ <para>
+ This option has no effect on the exclude table and schema patterns
+ (and also <option>--exclude-table-data</>): not matching any entities
+ isn't considered an error.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-T <replaceable class="parameter">table</replaceable></option></term>
<term><option>--exclude-table=<replaceable class="parameter">table</replaceable></option></term>
<listitem>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index 0e036b8..54618fa
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** static const char *username_subquery;
*** 97,102 ****
--- 97,105 ----
/* obsolete as of 7.3: */
static Oid g_last_builtin_oid; /* value of the last builtin oid */
+ /* The specified names/patterns should to match at least one entity */
+ static int strict_mode = 0;
+
/*
* Object inclusion/exclusion lists
*
*************** static void setup_connection(Archive *AH
*** 131,140 ****
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids);
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);
--- 134,145 ----
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict_mode);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict_mode);
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);
*************** main(int argc, char **argv)
*** 332,337 ****
--- 337,343 ----
{"section", required_argument, NULL, 5},
{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
{"snapshot", required_argument, NULL, 6},
+ {"strict-mode", no_argument, &strict_mode, 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},
*************** main(int argc, char **argv)
*** 684,710 ****
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
! &schema_include_oids);
if (schema_include_oids.head == NULL)
exit_horribly(NULL, "No matching schemas were found\n");
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
! &schema_exclude_oids);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
if (table_include_patterns.head != NULL)
{
expand_table_name_patterns(fout, &table_include_patterns,
! &table_include_oids);
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);
expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! &tabledata_exclude_oids);
/* non-matching exclusion patterns aren't an error */
--- 690,721 ----
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
! &schema_include_oids,
! strict_mode);
if (schema_include_oids.head == NULL)
exit_horribly(NULL, "No matching schemas were found\n");
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
! &schema_exclude_oids,
! false);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
if (table_include_patterns.head != NULL)
{
expand_table_name_patterns(fout, &table_include_patterns,
! &table_include_oids,
! strict_mode);
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,
! false);
expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! &tabledata_exclude_oids,
! false);
/* non-matching exclusion patterns aren't an error */
*************** help(const char *progname)
*** 899,904 ****
--- 910,917 ----
printf(_(" --section=SECTION dump named section (pre-data, data, or post-data)\n"));
printf(_(" --serializable-deferrable wait until the dump can run without anomalies\n"));
printf(_(" --snapshot=SNAPSHOT use given synchronous snapshot for the dump\n"));
+ printf(_(" --strict-mode require table and/or schema include patterns to\n"
+ " match at least one entity each\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
*************** parseArchiveFormat(const char *format, A
*** 1129,1135 ****
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids)
{
PQExpBuffer query;
PGresult *res;
--- 1142,1149 ----
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict_mode)
{
PQExpBuffer query;
PGresult *res;
*************** expand_schema_name_patterns(Archive *fou
*** 1145,1182 ****
query = createPQExpBuffer();
/*
! * We use UNION ALL rather than UNION; this might sometimes result in
! * duplicate entries in the OID list, but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
- if (cell != patterns->head)
- appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
processSQLNamePattern(GetConnection(fout), query, cell->val, false,
false, NULL, "n.nspname", NULL, NULL);
- }
! 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)));
}
- PQclear(res);
destroyPQExpBuffer(query);
}
/*
* 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)
{
PQExpBuffer query;
PGresult *res;
--- 1159,1199 ----
query = createPQExpBuffer();
/*
! * This might sometimes result in duplicate entries in the OID list,
! * but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
processSQLNamePattern(GetConnection(fout), query, cell->val, false,
false, NULL, "n.nspname", NULL, NULL);
! res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
! if (strict_mode && PQntuples(res) == 0)
! exit_horribly(NULL, "Schema \"%s\" not found.\n", cell->val);
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
!
! PQclear(res);
! resetPQExpBuffer(query);
}
destroyPQExpBuffer(query);
}
/*
* 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,
! bool strict_mode)
{
PQExpBuffer query;
PGresult *res;
*************** expand_table_name_patterns(Archive *fout
*** 1189,1202 ****
query = createPQExpBuffer();
/*
* We use UNION ALL rather than UNION; this might sometimes result in
* duplicate entries in the OID list, but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
! if (cell != patterns->head)
appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
--- 1206,1224 ----
query = createPQExpBuffer();
/*
+ * In this case we expect possibly longer list of patterns. If it is not
+ * necessary, we try to minimize number of SQL executions and try to build
+ * one larger query. But strict_mode requires immediate exec for any pattern.
+ *
* We use UNION ALL rather than UNION; this might sometimes result in
* duplicate entries in the OID list, but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
! if (!strict_mode && cell != patterns->head)
appendPQExpBufferStr(query, "UNION ALL\n");
+
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
*************** expand_table_name_patterns(Archive *fout
*** 1207,1222 ****
processSQLNamePattern(GetConnection(fout), query, cell->val, true,
false, "n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- }
! 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)));
}
- PQclear(res);
destroyPQExpBuffer(query);
}
--- 1229,1263 ----
processSQLNamePattern(GetConnection(fout), query, cell->val, true,
false, "n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
! if (strict_mode)
! {
! res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
! if (strict_mode && PQntuples(res) == 0)
! exit_horribly(NULL, "Table \"%s\" not found.\n", cell->val);
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
!
! PQclear(res);
! resetPQExpBuffer(query);
! }
! }
!
! if (!strict_mode)
{
! 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)));
! }
!
! PQclear(res);
}
destroyPQExpBuffer(query);
}
On 07/25/2015 07:08 PM, Pavel Stehule wrote:
I am sending a new patch - without checking wildcard chars.
The documentation says the option is called --strict-names, while the
code has --strict-mode. I like --strict-names more, "mode" seems
redundant, and it's not clear what it's strict about.
For symmetry, it would be good to also support this option in
pg_restore. It seems even more useful there.
Can we do better than issuing a separate query for each table/schema
name? The performance of this isn't very important, but still it seems
like you could fairly easily refactor the code to avoid that. Perhaps
return an extra constant for part of the UNION to distinguish which
result row came from which pattern, and check that at least one row is
returned for each.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
2015-07-30 12:44 GMT+02:00 Heikki Linnakangas <hlinnaka@iki.fi>:
On 07/25/2015 07:08 PM, Pavel Stehule wrote:
I am sending a new patch - without checking wildcard chars.
The documentation says the option is called --strict-names, while the code
has --strict-mode. I like --strict-names more, "mode" seems redundant, and
it's not clear what it's strict about.
ok
For symmetry, it would be good to also support this option in pg_restore.
It seems even more useful there.
I'll do it
Can we do better than issuing a separate query for each table/schema name?
The performance of this isn't very important, but still it seems like you
could fairly easily refactor the code to avoid that. Perhaps return an
extra constant for part of the UNION to distinguish which result row came
from which pattern, and check that at least one row is returned for each.
I did few tests and for 1K tables the union is faster about 50ms, but the
code is much more complex, for 10K tables, the union is significantly
slower (probably due planning) 2sec x 7sec. So if we are expecting backup
on not too slow network, then simple solution is winner - Postgres process
simple read queries quickly.
Regards
Pavel
Show quoted text
- Heikki
Hi
I am sending updated version
news:
* strict-names everywhere
* checking table names in pg_dump simplified - not necessary to create
single query
* pg_restore support
Regards
Pavel
2015-08-13 9:17 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
Show quoted text
Hi
2015-07-30 12:44 GMT+02:00 Heikki Linnakangas <hlinnaka@iki.fi>:
On 07/25/2015 07:08 PM, Pavel Stehule wrote:
I am sending a new patch - without checking wildcard chars.
The documentation says the option is called --strict-names, while the
code has --strict-mode. I like --strict-names more, "mode" seems redundant,
and it's not clear what it's strict about.ok
For symmetry, it would be good to also support this option in pg_restore.
It seems even more useful there.I'll do it
Can we do better than issuing a separate query for each table/schema
name? The performance of this isn't very important, but still it seems like
you could fairly easily refactor the code to avoid that. Perhaps return an
extra constant for part of the UNION to distinguish which result row came
from which pattern, and check that at least one row is returned for each.I did few tests and for 1K tables the union is faster about 50ms, but the
code is much more complex, for 10K tables, the union is significantly
slower (probably due planning) 2sec x 7sec. So if we are expecting backup
on not too slow network, then simple solution is winner - Postgres process
simple read queries quickly.Regards
Pavel
- Heikki
Attachments:
pg_dump-strict-names-7.patchtext/x-patch; charset=US-ASCII; name=pg_dump-strict-names-7.patchDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
new file mode 100644
index 7467e86..7c071fb
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*************** PostgreSQL documentation
*** 545,550 ****
--- 545,566 ----
</varlistentry>
<varlistentry>
+ <term><option>--strict-names</></term>
+ <listitem>
+ <para>
+ Require that table and/or schema match at least one entity each.
+ Without any entity in the database to be dumped, an error message
+ is printed and dump is aborted.
+ </para>
+ <para>
+ This option has no effect on the exclude table and schema patterns
+ (and also <option>--exclude-table-data</>): not matching any entities
+ isn't considered an error.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-T <replaceable class="parameter">table</replaceable></option></term>
<term><option>--exclude-table=<replaceable class="parameter">table</replaceable></option></term>
<listitem>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
new file mode 100644
index 97e3420..9df7f69
*** a/doc/src/sgml/ref/pg_restore.sgml
--- b/doc/src/sgml/ref/pg_restore.sgml
***************
*** 432,437 ****
--- 432,448 ----
</varlistentry>
<varlistentry>
+ <term><option>--strict-names</></term>
+ <listitem>
+ <para>
+ Require that table and/or schema and/or function and/or trigger match
+ at least one entity each. Without any entity in the backup file,
+ an error message is printed and restore is aborted.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-T <replaceable class="parameter">trigger</replaceable></option></term>
<term><option>--trigger=<replaceable class="parameter">trigger</replaceable></option></term>
<listitem>
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
new file mode 100644
index d7506e1..52b2b98
*** a/src/bin/pg_dump/dumputils.c
--- b/src/bin/pg_dump/dumputils.c
*************** simple_string_list_append(SimpleStringLi
*** 1220,1225 ****
--- 1220,1226 ----
pg_malloc(offsetof(SimpleStringListCell, val) +strlen(val) + 1);
cell->next = NULL;
+ cell->touched = false;
strcpy(cell->val, val);
if (list->tail)
*************** simple_string_list_member(SimpleStringLi
*** 1237,1243 ****
--- 1238,1260 ----
for (cell = list->head; cell; cell = cell->next)
{
if (strcmp(cell->val, val) == 0)
+ {
+ cell->touched = true;
return true;
+ }
}
return false;
}
+
+ const char *
+ simple_string_list_not_touched(SimpleStringList *list)
+ {
+ SimpleStringListCell *cell;
+
+ for (cell = list->head; cell; cell = cell->next)
+ {
+ if (!cell->touched)
+ return cell->val;
+ }
+ return NULL;
+ }
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
new file mode 100644
index b176746..9f31bbc
*** a/src/bin/pg_dump/dumputils.h
--- b/src/bin/pg_dump/dumputils.h
*************** typedef struct SimpleOidList
*** 38,43 ****
--- 38,45 ----
typedef struct SimpleStringListCell
{
struct SimpleStringListCell *next;
+ bool touched; /* true, when this string was searched
+ and touched */
char val[FLEXIBLE_ARRAY_MEMBER]; /* null-terminated string here */
} SimpleStringListCell;
*************** extern void set_dump_section(const char
*** 103,107 ****
--- 105,111 ----
extern void simple_string_list_append(SimpleStringList *list, const char *val);
extern bool simple_string_list_member(SimpleStringList *list, const char *val);
+ extern const char *simple_string_list_not_touched(SimpleStringList *list);
+
#endif /* DUMPUTILS_H */
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
new file mode 100644
index 80df8fc..7126749
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
*************** typedef struct _restoreOptions
*** 104,109 ****
--- 104,110 ----
int column_inserts;
int if_exists;
int no_security_labels; /* Skip security label entries */
+ int strict_names;
const char *filename;
int dataOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index 8f1f6c1..2344937
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** static void reduce_dependencies(ArchiveH
*** 108,113 ****
--- 108,116 ----
static void mark_create_done(ArchiveHandle *AH, TocEntry *te);
static void inhibit_data_for_failed_table(ArchiveHandle *AH, TocEntry *te);
+ static void StrictNamesCheck(RestoreOptions *ropt);
+
+
/*
* Allocate a new DumpOptions block containing all default values.
*/
*************** SetArchiveRestoreOptions(Archive *AHX, R
*** 284,289 ****
--- 287,296 ----
te->reqs = _tocEntryRequired(te, curSection, ropt);
}
+
+ /* Enforce strict names checking */
+ if (ropt->strict_names)
+ StrictNamesCheck(ropt);
}
/* Public */
*************** PrintTOCSummary(Archive *AHX, RestoreOpt
*** 1104,1109 ****
--- 1111,1120 ----
}
}
+ /* Enforce strict names checking */
+ if (ropt->strict_names)
+ StrictNamesCheck(ropt);
+
if (ropt->filename)
RestoreOutput(AH, sav);
}
*************** processStdStringsEntry(ArchiveHandle *AH
*** 2612,2617 ****
--- 2623,2671 ----
te->defn);
}
+ static void
+ StrictNamesCheck(RestoreOptions *ropt)
+ {
+ const char *missing_name;
+
+ Assert(ropt->strict_names);
+
+ if (ropt->schemaNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->schemaNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Schema \"%s\" not found.\n", missing_name);
+ }
+
+ if (ropt->tableNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->tableNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Table \"%s\" not found.\n", missing_name);
+ }
+
+ if (ropt->indexNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->indexNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Index \"%s\" not found.\n", missing_name);
+ }
+
+ if (ropt->functionNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->functionNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Function \"%s\" not found.\n", missing_name);
+ }
+
+ if (ropt->triggerNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->triggerNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Trigger \"%s\" not found.\n", missing_name);
+ }
+ }
+
static teReqs
_tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index 87dadbf..3914cfb
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** static const char *username_subquery;
*** 97,102 ****
--- 97,105 ----
/* obsolete as of 7.3: */
static Oid g_last_builtin_oid; /* value of the last builtin oid */
+ /* The specified names/patterns should to match at least one entity */
+ static int strict_names = 0;
+
/*
* Object inclusion/exclusion lists
*
*************** static void setup_connection(Archive *AH
*** 131,140 ****
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids);
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);
--- 134,145 ----
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict_names);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict_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);
*************** main(int argc, char **argv)
*** 333,338 ****
--- 338,344 ----
{"section", required_argument, NULL, 5},
{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
{"snapshot", required_argument, NULL, 6},
+ {"strict-mode", no_argument, &strict_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},
*************** main(int argc, char **argv)
*** 690,716 ****
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
! &schema_include_oids);
if (schema_include_oids.head == NULL)
exit_horribly(NULL, "No matching schemas were found\n");
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
! &schema_exclude_oids);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
if (table_include_patterns.head != NULL)
{
expand_table_name_patterns(fout, &table_include_patterns,
! &table_include_oids);
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);
expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! &tabledata_exclude_oids);
/* non-matching exclusion patterns aren't an error */
--- 696,727 ----
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
! &schema_include_oids,
! strict_names);
if (schema_include_oids.head == NULL)
exit_horribly(NULL, "No matching schemas were found\n");
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
! &schema_exclude_oids,
! false);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
if (table_include_patterns.head != NULL)
{
expand_table_name_patterns(fout, &table_include_patterns,
! &table_include_oids,
! strict_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,
! false);
expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! &tabledata_exclude_oids,
! false);
/* non-matching exclusion patterns aren't an error */
*************** help(const char *progname)
*** 905,910 ****
--- 916,923 ----
printf(_(" --section=SECTION dump named section (pre-data, data, or post-data)\n"));
printf(_(" --serializable-deferrable wait until the dump can run without anomalies\n"));
printf(_(" --snapshot=SNAPSHOT use given synchronous snapshot for the dump\n"));
+ printf(_(" --strict-names require table and/or schema include patterns to\n"
+ " match at least one entity each\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
*************** parseArchiveFormat(const char *format, A
*** 1135,1141 ****
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids)
{
PQExpBuffer query;
PGresult *res;
--- 1148,1155 ----
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict_names)
{
PQExpBuffer query;
PGresult *res;
*************** expand_schema_name_patterns(Archive *fou
*** 1151,1188 ****
query = createPQExpBuffer();
/*
! * We use UNION ALL rather than UNION; this might sometimes result in
! * duplicate entries in the OID list, but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
- if (cell != patterns->head)
- appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
processSQLNamePattern(GetConnection(fout), query, cell->val, false,
false, NULL, "n.nspname", NULL, NULL);
- }
! 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)));
}
- PQclear(res);
destroyPQExpBuffer(query);
}
/*
* 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)
{
PQExpBuffer query;
PGresult *res;
--- 1165,1205 ----
query = createPQExpBuffer();
/*
! * This might sometimes result in duplicate entries in the OID list,
! * but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
processSQLNamePattern(GetConnection(fout), query, cell->val, false,
false, NULL, "n.nspname", NULL, NULL);
! res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
! if (strict_names && PQntuples(res) == 0)
! exit_horribly(NULL, "Schema \"%s\" not found.\n", cell->val);
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
!
! PQclear(res);
! resetPQExpBuffer(query);
}
destroyPQExpBuffer(query);
}
/*
* 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,
! bool strict_names)
{
PQExpBuffer query;
PGresult *res;
*************** expand_table_name_patterns(Archive *fout
*** 1195,1208 ****
query = createPQExpBuffer();
/*
! * We use UNION ALL rather than UNION; this might sometimes result in
! * duplicate entries in the OID list, but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
- if (cell != patterns->head)
- appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
--- 1212,1223 ----
query = createPQExpBuffer();
/*
! * this might sometimes result in duplicate entries in the OID list,
! * but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
*************** expand_table_name_patterns(Archive *fout
*** 1213,1228 ****
processSQLNamePattern(GetConnection(fout), query, cell->val, true,
false, "n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- }
! 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)));
}
- PQclear(res);
destroyPQExpBuffer(query);
}
--- 1228,1247 ----
processSQLNamePattern(GetConnection(fout), query, cell->val, true,
false, "n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
! res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
! if (strict_names && PQntuples(res) == 0)
! exit_horribly(NULL, "Table \"%s\" not found.\n", cell->val);
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
!
! PQclear(res);
! resetPQExpBuffer(query);
}
destroyPQExpBuffer(query);
}
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
new file mode 100644
index b129488..75c08b9
*** a/src/bin/pg_dump/pg_restore.c
--- b/src/bin/pg_dump/pg_restore.c
*************** main(int argc, char **argv)
*** 77,82 ****
--- 77,83 ----
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
static int no_security_labels = 0;
+ static int strict_names = 0;
struct option cmdopts[] = {
{"clean", 0, NULL, 'c'},
*************** main(int argc, char **argv)
*** 118,123 ****
--- 119,125 ----
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
{"section", required_argument, NULL, 3},
+ {"strict-names", no_argument, &strict_names, 1},
{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
{"no-security-labels", no_argument, &no_security_labels, 1},
*************** main(int argc, char **argv)
*** 345,350 ****
--- 347,353 ----
exit_nicely(1);
}
opts->if_exists = if_exists;
+ opts->strict_names = strict_names;
if (opts->formatName)
{
*************** usage(const char *progname)
*** 467,472 ****
--- 470,477 ----
printf(_(" --no-security-labels do not restore security labels\n"));
printf(_(" --no-tablespaces do not restore tablespace assignments\n"));
printf(_(" --section=SECTION restore named section (pre-data, data, or post-data)\n"));
+ printf(_(" --strict-names require table and/or schema include patterns to\n"
+ " match at least one entity each\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, failed
Spec compliant: not tested
Documentation: not tested
The feature doesn't seem to work:
pg_dump -t t -t 'ii*' --strict-names
pg_dump: unrecognized option `--strict-names'
Try "pg_dump --help" for more information.
decibel@decina:[16:58]~/git/postgres/i (pg_dump-strict-names-7.patch=)$bin/p
The documentation could use some improvements.
+ <para>
+ Require that table and/or schema match at least one entity each.
+ Without any entity in the database to be dumped, an error message
+ is printed and dump is aborted.
+ </para>
Would be clearer as
Require that each schema (-n / --schema) and table (-t / --table) qualifier match at least one schema/table in the database to be dumped. Note that if none of the schema/table qualifiers find matches pg_dump will generate an error even without --strict-names.
+ <para>
+ This option has no effect on the exclude table and schema patterns
+ (and also <option>--exclude-table-data</>): not matching any entities
+ isn't considered an error.
Rewrite:
This option has no effect on -N/--exclude-schema, -T/--exclude_table or --exclude-table-date. An exclude pattern failing to match any objects is not considered an error.
The new status of this patch is: Waiting on Author
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
2015-08-22 0:09 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, failed
Spec compliant: not tested
Documentation: not testedThe feature doesn't seem to work:
pg_dump -t t -t 'ii*' --strict-names
pg_dump: unrecognized option `--strict-names'
Try "pg_dump --help" for more information.
decibel@decina:[16:58]~/git/postgres/i
(pg_dump-strict-names-7.patch=)$bin/p
sorry - there was wrong strict-mode
fixed
The documentation could use some improvements.
+ <para> + Require that table and/or schema match at least one entity each. + Without any entity in the database to be dumped, an error message + is printed and dump is aborted. + </para>Would be clearer as
Require that each schema (-n / --schema) and table (-t / --table)
qualifier match at least one schema/table in the database to be dumped.
Note that if none of the schema/table qualifiers find matches pg_dump will
generate an error even without --strict-names.+ <para> + This option has no effect on the exclude table and schema patterns + (and also <option>--exclude-table-data</>): not matching any entities + isn't considered an error.Rewrite:
This option has no effect on -N/--exclude-schema, -T/--exclude_table or
--exclude-table-date. An exclude pattern failing to match any objects is
not considered an error.
fixed
Regards
Pavel
Show quoted text
The new status of this patch is: Waiting on Author
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
pg_dump-strict-names-8.patchtext/x-patch; charset=US-ASCII; name=pg_dump-strict-names-8.patchDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 7467e86..eaa006d 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -545,6 +545,23 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--strict-names</></term>
+ <listitem>
+ <para>
+ Require that each schema (-n / --schema) and table (-t / --table)
+ qualifier match at least one schema/table in the database to be dumped.
+ Note that if none of the schema/table qualifiers find matches pg_dump
+ will generate an error even without --strict-names.
+ </para>
+ <para>
+ This option has no effect on -N/--exclude-schema, -T/--exclude_table
+ or --exclude-table-date. An exclude pattern failing to match
+ any bjects is not considered an error.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-T <replaceable class="parameter">table</replaceable></option></term>
<term><option>--exclude-table=<replaceable class="parameter">table</replaceable></option></term>
<listitem>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 97e3420..a5a9394 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -432,6 +432,16 @@
</varlistentry>
<varlistentry>
+ <term><option>--strict-names</></term>
+ <listitem>
+ <para>
+ Require that each schema (-n / --schema) and table (-t / --table)
+ qualifier match at least one schema/table in the backup file.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-T <replaceable class="parameter">trigger</replaceable></option></term>
<term><option>--trigger=<replaceable class="parameter">trigger</replaceable></option></term>
<listitem>
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index d7506e1..52b2b98 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -1220,6 +1220,7 @@ simple_string_list_append(SimpleStringList *list, const char *val)
pg_malloc(offsetof(SimpleStringListCell, val) +strlen(val) + 1);
cell->next = NULL;
+ cell->touched = false;
strcpy(cell->val, val);
if (list->tail)
@@ -1237,7 +1238,23 @@ simple_string_list_member(SimpleStringList *list, const char *val)
for (cell = list->head; cell; cell = cell->next)
{
if (strcmp(cell->val, val) == 0)
+ {
+ cell->touched = true;
return true;
+ }
}
return false;
}
+
+const char *
+simple_string_list_not_touched(SimpleStringList *list)
+{
+ SimpleStringListCell *cell;
+
+ for (cell = list->head; cell; cell = cell->next)
+ {
+ if (!cell->touched)
+ return cell->val;
+ }
+ return NULL;
+}
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index b176746..9f31bbc 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -38,6 +38,8 @@ typedef struct SimpleOidList
typedef struct SimpleStringListCell
{
struct SimpleStringListCell *next;
+ bool touched; /* true, when this string was searched
+ and touched */
char val[FLEXIBLE_ARRAY_MEMBER]; /* null-terminated string here */
} SimpleStringListCell;
@@ -103,5 +105,7 @@ extern void set_dump_section(const char *arg, int *dumpSections);
extern void simple_string_list_append(SimpleStringList *list, const char *val);
extern bool simple_string_list_member(SimpleStringList *list, const char *val);
+extern const char *simple_string_list_not_touched(SimpleStringList *list);
+
#endif /* DUMPUTILS_H */
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 80df8fc..7126749 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -104,6 +104,7 @@ typedef struct _restoreOptions
int column_inserts;
int if_exists;
int no_security_labels; /* Skip security label entries */
+ int strict_names;
const char *filename;
int dataOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 8f1f6c1..2344937 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -108,6 +108,9 @@ static void reduce_dependencies(ArchiveHandle *AH, TocEntry *te,
static void mark_create_done(ArchiveHandle *AH, TocEntry *te);
static void inhibit_data_for_failed_table(ArchiveHandle *AH, TocEntry *te);
+static void StrictNamesCheck(RestoreOptions *ropt);
+
+
/*
* Allocate a new DumpOptions block containing all default values.
*/
@@ -284,6 +287,10 @@ SetArchiveRestoreOptions(Archive *AHX, RestoreOptions *ropt)
te->reqs = _tocEntryRequired(te, curSection, ropt);
}
+
+ /* Enforce strict names checking */
+ if (ropt->strict_names)
+ StrictNamesCheck(ropt);
}
/* Public */
@@ -1104,6 +1111,10 @@ PrintTOCSummary(Archive *AHX, RestoreOptions *ropt)
}
}
+ /* Enforce strict names checking */
+ if (ropt->strict_names)
+ StrictNamesCheck(ropt);
+
if (ropt->filename)
RestoreOutput(AH, sav);
}
@@ -2612,6 +2623,49 @@ processStdStringsEntry(ArchiveHandle *AH, TocEntry *te)
te->defn);
}
+static void
+StrictNamesCheck(RestoreOptions *ropt)
+{
+ const char *missing_name;
+
+ Assert(ropt->strict_names);
+
+ if (ropt->schemaNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->schemaNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Schema \"%s\" not found.\n", missing_name);
+ }
+
+ if (ropt->tableNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->tableNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Table \"%s\" not found.\n", missing_name);
+ }
+
+ if (ropt->indexNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->indexNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Index \"%s\" not found.\n", missing_name);
+ }
+
+ if (ropt->functionNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->functionNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Function \"%s\" not found.\n", missing_name);
+ }
+
+ if (ropt->triggerNames.head != NULL)
+ {
+ missing_name = simple_string_list_not_touched(&ropt->triggerNames);
+ if (missing_name != NULL)
+ exit_horribly(modulename, "Trigger \"%s\" not found.\n", missing_name);
+ }
+}
+
static teReqs
_tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 87dadbf..f98dde1 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -97,6 +97,9 @@ static const char *username_subquery;
/* obsolete as of 7.3: */
static Oid g_last_builtin_oid; /* value of the last builtin oid */
+/* The specified names/patterns should to match at least one entity */
+static int strict_names = 0;
+
/*
* Object inclusion/exclusion lists
*
@@ -131,10 +134,12 @@ static void setup_connection(Archive *AH, DumpOptions *dopt,
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids);
+ SimpleOidList *oids,
+ bool strict_names);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids);
+ SimpleOidList *oids,
+ bool strict_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);
@@ -333,6 +338,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-names", no_argument, &strict_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},
@@ -690,27 +696,32 @@ main(int argc, char **argv)
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
- &schema_include_oids);
+ &schema_include_oids,
+ strict_names);
if (schema_include_oids.head == NULL)
exit_horribly(NULL, "No matching schemas were found\n");
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
- &schema_exclude_oids);
+ &schema_exclude_oids,
+ false);
/* non-matching exclusion patterns aren't an error */
/* Expand table selection patterns into OID lists */
if (table_include_patterns.head != NULL)
{
expand_table_name_patterns(fout, &table_include_patterns,
- &table_include_oids);
+ &table_include_oids,
+ strict_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 */
@@ -905,6 +916,8 @@ help(const char *progname)
printf(_(" --section=SECTION dump named section (pre-data, data, or post-data)\n"));
printf(_(" --serializable-deferrable wait until the dump can run without anomalies\n"));
printf(_(" --snapshot=SNAPSHOT use given synchronous snapshot for the dump\n"));
+ printf(_(" --strict-names require table and/or schema include patterns to\n"
+ " match at least one entity each\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
@@ -1135,7 +1148,8 @@ parseArchiveFormat(const char *format, ArchiveMode *mode)
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids)
+ SimpleOidList *oids,
+ bool strict_names)
{
PQExpBuffer query;
PGresult *res;
@@ -1151,38 +1165,41 @@ expand_schema_name_patterns(Archive *fout,
query = createPQExpBuffer();
/*
- * We use UNION ALL rather than UNION; this might sometimes result in
- * duplicate entries in the OID list, but we don't care.
+ * This might sometimes result in duplicate entries in the OID list,
+ * but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
- if (cell != patterns->head)
- appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT oid FROM pg_catalog.pg_namespace n\n");
processSQLNamePattern(GetConnection(fout), query, cell->val, false,
false, NULL, "n.nspname", NULL, NULL);
- }
- res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (strict_names && PQntuples(res) == 0)
+ exit_horribly(NULL, "Schema \"%s\" not found.\n", cell->val);
- 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);
+ resetPQExpBuffer(query);
}
- PQclear(res);
destroyPQExpBuffer(query);
}
/*
* Find the OIDs of all tables matching the given list of patterns,
- * and append them to the given OID list.
+ * 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_names)
{
PQExpBuffer query;
PGresult *res;
@@ -1195,14 +1212,12 @@ expand_table_name_patterns(Archive *fout,
query = createPQExpBuffer();
/*
- * We use UNION ALL rather than UNION; this might sometimes result in
- * duplicate entries in the OID list, but we don't care.
+ * this might sometimes result in duplicate entries in the OID list,
+ * but we don't care.
*/
for (cell = patterns->head; cell; cell = cell->next)
{
- if (cell != patterns->head)
- appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
@@ -1213,16 +1228,20 @@ expand_table_name_patterns(Archive *fout,
processSQLNamePattern(GetConnection(fout), query, cell->val, true,
false, "n.nspname", "c.relname", NULL,
"pg_catalog.pg_table_is_visible(c.oid)");
- }
- res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (strict_names && PQntuples(res) == 0)
+ exit_horribly(NULL, "Table \"%s\" not found.\n", cell->val);
- 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);
+ resetPQExpBuffer(query);
}
- PQclear(res);
destroyPQExpBuffer(query);
}
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index b129488..75c08b9 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -77,6 +77,7 @@ main(int argc, char **argv)
static int outputNoTablespaces = 0;
static int use_setsessauth = 0;
static int no_security_labels = 0;
+ static int strict_names = 0;
struct option cmdopts[] = {
{"clean", 0, NULL, 'c'},
@@ -118,6 +119,7 @@ main(int argc, char **argv)
{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
{"role", required_argument, NULL, 2},
{"section", required_argument, NULL, 3},
+ {"strict-names", no_argument, &strict_names, 1},
{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
{"no-security-labels", no_argument, &no_security_labels, 1},
@@ -345,6 +347,7 @@ main(int argc, char **argv)
exit_nicely(1);
}
opts->if_exists = if_exists;
+ opts->strict_names = strict_names;
if (opts->formatName)
{
@@ -467,6 +470,8 @@ usage(const char *progname)
printf(_(" --no-security-labels do not restore security labels\n"));
printf(_(" --no-tablespaces do not restore tablespace assignments\n"));
printf(_(" --section=SECTION restore named section (pre-data, data, or post-data)\n"));
+ printf(_(" --strict-names require table and/or schema include patterns to\n"
+ " match at least one entity each\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
On Sun, Aug 23, 2015 at 10:47 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
[blah]
fixed
Moved to next CF 2015-09.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers